From 24430131ec15b55b4e13412abb5676f2c15d5b69 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 18 Oct 2024 16:12:28 +0700 Subject: [PATCH 1/8] Handle reset CoreDataManager in the background --- .../Storage/CoreData/CoreDataManager.swift | 25 ++++++++++--------- .../CoreData/CoreDataManagerTests.swift | 19 +++++++++----- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index a0868abeff1..d2d384527dd 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -203,20 +203,22 @@ public final class CoreDataManager: StorageManagerType { /// This method effectively destroys all of the stored data, and generates a blank Persistent Store from scratch. /// public func reset() { - let viewContext = persistentContainer.viewContext - - viewContext.performAndWait { - viewContext.reset() - self.deleteAllStoredObjects() - + performAndSave({ storage in + guard let backgroundContext = storage as? NSManagedObjectContext else { + DDLogError("⛔️ CoreDataManager failed to reset due to unexpected storage type!") + return + } + /// persist self to complete deleting objects + self.deleteAllStoredObjects(in: backgroundContext) + backgroundContext.reset() + }, completion: { DDLogVerbose("💣 [CoreDataManager] Stack Destroyed!") NotificationCenter.default.post(name: .StorageManagerDidResetStorage, object: self) - } + }, on: .main) } - private func deleteAllStoredObjects() { + private func deleteAllStoredObjects(in context: NSManagedObjectContext) { let storeCoordinator = persistentContainer.persistentStoreCoordinator - let viewContext = persistentContainer.viewContext do { let entities = storeCoordinator.managedObjectModel.entities for entity in entities { @@ -224,11 +226,10 @@ public final class CoreDataManager: StorageManagerType { continue } let fetchRequest = NSFetchRequest(entityName: entityName) - let objects = try viewContext.fetch(fetchRequest) as? [NSManagedObject] + let objects = try context.fetch(fetchRequest) as? [NSManagedObject] objects?.forEach { object in - viewContext.delete(object) + context.delete(object) } - viewContext.saveIfNeeded() } } catch { logErrorAndExit("☠️ [CoreDataManager] Cannot delete stored objects! \(error)") diff --git a/Storage/StorageTests/CoreData/CoreDataManagerTests.swift b/Storage/StorageTests/CoreData/CoreDataManagerTests.swift index 07b829bb150..631c18750ad 100644 --- a/Storage/StorageTests/CoreData/CoreDataManagerTests.swift +++ b/Storage/StorageTests/CoreData/CoreDataManagerTests.swift @@ -59,15 +59,22 @@ final class CoreDataManagerTests: XCTestCase { let manager = CoreDataManager(name: storageIdentifier, crashLogger: MockCrashLogger()) manager.reset() let viewContext = try XCTUnwrap(manager.viewStorage as? NSManagedObjectContext) - _ = viewContext.insertNewObject(ofType: ShippingLine.self) - viewContext.saveIfNeeded() - XCTAssertEqual(viewContext.countObjects(ofType: ShippingLine.self), 1) // Action - manager.reset() + waitForExpectation { expectation in + manager.performAndSave({ storage in + _ = storage.insertNewObject(ofType: ShippingLine.self) + }, completion: { + XCTAssertEqual(viewContext.countObjects(ofType: ShippingLine.self), 1) + manager.reset() + expectation.fulfill() + }, on: .main) + } // Assert - XCTAssertEqual(viewContext.countObjects(ofType: ShippingLine.self), 0) + waitUntil { + viewContext.countObjects(ofType: ShippingLine.self) == 0 + } } func test_performAndSave_executes_changes_in_background_then_updates_viewContext() throws { @@ -78,7 +85,7 @@ final class CoreDataManagerTests: XCTestCase { XCTAssertEqual(viewContext.countObjects(ofType: Account.self), 0) // Action - waitForExpectation(count: 1) { expectation in + waitForExpectation { expectation in manager.performAndSave({ storage in XCTAssertFalse(Thread.current.isMainThread, "Write operations should be performed in the background.") self.insertAccount(to: storage) From 06806908035aeed1cb8662275fece56b4bf8b8a7 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 18 Oct 2024 16:21:01 +0700 Subject: [PATCH 2/8] Avoid reloading data on the dashboard when logging out --- .../Blaze/BlazeLocalNotificationScheduler.swift | 8 +++++++- .../Blaze/BlazeCampaignDashboardViewModel.swift | 10 ++++++++-- .../ViewRelated/Dashboard/DashboardViewModel.swift | 8 +++++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Classes/Blaze/BlazeLocalNotificationScheduler.swift b/WooCommerce/Classes/Blaze/BlazeLocalNotificationScheduler.swift index bfbb6b92111..53ac082ab8c 100644 --- a/WooCommerce/Classes/Blaze/BlazeLocalNotificationScheduler.swift +++ b/WooCommerce/Classes/Blaze/BlazeLocalNotificationScheduler.swift @@ -159,7 +159,13 @@ private extension DefaultBlazeLocalNotificationScheduler { self?.scheduleNoCampaignReminderIfNeeded() } blazeCampaignResultsController.onDidResetContent = { [weak self] in - self?.scheduleNoCampaignReminderIfNeeded() + /// Upon logging out, `CoreDataManager` resets the storage and triggers the reset notification + /// causing refetching data. Checking the authentication state helps avoiding reloading data + /// in the unauthenticated state. + guard let self, stores.isAuthenticated else { + return + } + scheduleNoCampaignReminderIfNeeded() } do { diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift index b1db5c6ac83..9ca73f602a6 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift @@ -319,8 +319,14 @@ private extension BlazeCampaignDashboardViewModel { self?.updateResults() } productResultsController.onDidResetContent = { [weak self] in - self?.updateAvailability() - self?.updateResults() + /// Upon logging out, `CoreDataManager` resets the storage and triggers the reset notification + /// causing refetching data. Checking the authentication state helps avoiding reloading data + /// in the unauthenticated state. + guard let self, stores.isAuthenticated else { + return + } + updateAvailability() + updateResults() } do { diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift index 4d048660106..c6425082298 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift @@ -529,7 +529,13 @@ private extension DashboardViewModel { ordersResultsController.onDidChangeContent = { refreshHasOrders() } - ordersResultsController.onDidResetContent = { + ordersResultsController.onDidResetContent = { [weak self] in + /// Upon logging out, `CoreDataManager` resets the storage and triggers the reset notification + /// causing refetching data. Checking the authentication state helps avoiding reloading data + /// in the unauthenticated state. + guard let self, stores.isAuthenticated else { + return + } refreshHasOrders() } From 2f9b95acf25e6f40c388ddc8f47c36e3bd793fa1 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 18 Oct 2024 16:42:16 +0700 Subject: [PATCH 3/8] Check authentication state before reloading dashboard cards --- .../Blaze/BlazeCampaignDashboardViewModel.swift | 10 ++-------- .../ViewRelated/Dashboard/DashboardViewModel.swift | 4 +++- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift index 9ca73f602a6..b1db5c6ac83 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift @@ -319,14 +319,8 @@ private extension BlazeCampaignDashboardViewModel { self?.updateResults() } productResultsController.onDidResetContent = { [weak self] in - /// Upon logging out, `CoreDataManager` resets the storage and triggers the reset notification - /// causing refetching data. Checking the authentication state helps avoiding reloading data - /// in the unauthenticated state. - guard let self, stores.isAuthenticated else { - return - } - updateAvailability() - updateResults() + self?.updateAvailability() + self?.updateResults() } do { diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift index c6425082298..d393c67a3be 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift @@ -482,7 +482,9 @@ private extension DashboardViewModel { $isEligibleForInbox) .receive(on: DispatchQueue.main) .sink { [weak self] combinedResult in - guard let self else { return } + guard let self, stores.isAuthenticated else { + return + } let ((canShowOnboarding, canShowBlaze), canShowGoogle, hasOrders, isEligibleForInbox) = combinedResult updateDashboardCards(canShowOnboarding: canShowOnboarding, canShowBlaze: canShowBlaze, From 0c8f5f086f68733d91d985ae89dc5771720e4b33 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 18 Oct 2024 16:43:04 +0700 Subject: [PATCH 4/8] Reset sessionManager before updating state to avoid leaving unauthenticated state with default store --- WooCommerce/Classes/Yosemite/DefaultStoresManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index 7c84cec6c61..8f315ef73ab 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -248,9 +248,9 @@ class DefaultStoresManager: StoresManager { let resetAction = CardPresentPaymentAction.reset dispatch(resetAction) + sessionManager.reset() state = DeauthenticatedState() - sessionManager.reset() ServiceLocator.analytics.refreshUserData() ZendeskProvider.shared.reset() ServiceLocator.storageManager.reset() From 75c4f9a4badb78fd815268b95257d616be3b8a51 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Mon, 21 Oct 2024 08:52:01 +0700 Subject: [PATCH 5/8] Revert "Check authentication state before reloading dashboard cards" This reverts commit 2f9b95acf25e6f40c388ddc8f47c36e3bd793fa1. --- .../Blaze/BlazeCampaignDashboardViewModel.swift | 10 ++++++++-- .../ViewRelated/Dashboard/DashboardViewModel.swift | 4 +--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift index b1db5c6ac83..9ca73f602a6 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift @@ -319,8 +319,14 @@ private extension BlazeCampaignDashboardViewModel { self?.updateResults() } productResultsController.onDidResetContent = { [weak self] in - self?.updateAvailability() - self?.updateResults() + /// Upon logging out, `CoreDataManager` resets the storage and triggers the reset notification + /// causing refetching data. Checking the authentication state helps avoiding reloading data + /// in the unauthenticated state. + guard let self, stores.isAuthenticated else { + return + } + updateAvailability() + updateResults() } do { diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift index d393c67a3be..c6425082298 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift @@ -482,9 +482,7 @@ private extension DashboardViewModel { $isEligibleForInbox) .receive(on: DispatchQueue.main) .sink { [weak self] combinedResult in - guard let self, stores.isAuthenticated else { - return - } + guard let self else { return } let ((canShowOnboarding, canShowBlaze), canShowGoogle, hasOrders, isEligibleForInbox) = combinedResult updateDashboardCards(canShowOnboarding: canShowOnboarding, canShowBlaze: canShowBlaze, From 92c770e634025e4cef97a4d05b5ec4775ef7a4bf Mon Sep 17 00:00:00 2001 From: Huong Do Date: Mon, 21 Oct 2024 09:19:16 +0700 Subject: [PATCH 6/8] Fix failed unit tests --- .../ViewRelated/Dashboard/DashboardViewModelTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift index d89aa5a51db..de9dcd24dfa 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift @@ -30,7 +30,7 @@ final class DashboardViewModelTests: XCTestCase { override func setUpWithError() throws { analyticsProvider = MockAnalyticsProvider() analytics = WooAnalytics(analyticsProvider: analyticsProvider) - stores = MockStoresManager(sessionManager: .makeForTesting()) + stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true)) userDefaults = try XCTUnwrap(UserDefaults(suiteName: "DashboardViewModelTests")) storageManager = MockStorageManager() } From 559a87089c0b8c62afd292a845299189b019c9d1 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Mon, 21 Oct 2024 09:26:46 +0700 Subject: [PATCH 7/8] Revert change on BlazeCampaignDashboardViewModel --- .../Blaze/BlazeCampaignDashboardViewModel.swift | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift index 9ca73f602a6..b1db5c6ac83 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Blaze/BlazeCampaignDashboardViewModel.swift @@ -319,14 +319,8 @@ private extension BlazeCampaignDashboardViewModel { self?.updateResults() } productResultsController.onDidResetContent = { [weak self] in - /// Upon logging out, `CoreDataManager` resets the storage and triggers the reset notification - /// causing refetching data. Checking the authentication state helps avoiding reloading data - /// in the unauthenticated state. - guard let self, stores.isAuthenticated else { - return - } - updateAvailability() - updateResults() + self?.updateAvailability() + self?.updateResults() } do { From 4e42ec31c8d05e75a450049fa5022ff35ecd0298 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Mon, 21 Oct 2024 10:17:32 +0700 Subject: [PATCH 8/8] Avoid triggering data reload upon logout --- .../Onboarding/StoreOnboardingViewModel.swift | 12 ++++++++++-- .../Classes/Yosemite/DefaultStoresManager.swift | 8 +++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Onboarding/StoreOnboardingViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Onboarding/StoreOnboardingViewModel.swift index 0c1b52cf665..a982337444a 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Onboarding/StoreOnboardingViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Onboarding/StoreOnboardingViewModel.swift @@ -97,8 +97,16 @@ class StoreOnboardingViewModel: ObservableObject { $noTasksAvailableForDisplay .combineLatest(defaults.publisher(for: \.completedAllStoreOnboardingTasks)) - .map { !($0 || $1) } - .assign(to: &$canShowInDashboard) + .filter { [weak self] _ in + /// Upon logging out, `UserDefaults` is reset causing `completedAllStoreOnboardingTasks` to change value. + /// Checking the authentication state helps avoiding reloading data in the unauthenticated state. + guard let self, self.stores.isAuthenticated else { + return false + } + return true + } + .map { !($0 || $1) } + .assign(to: &$canShowInDashboard) } func reloadTasks() async { diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index 8f315ef73ab..a842e9bc7df 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -245,11 +245,13 @@ class DefaultStoresManager: StoresManager { func deauthenticate() -> StoresManager { applicationPasswordGenerationFailureObserver = nil - let resetAction = CardPresentPaymentAction.reset - dispatch(resetAction) + if isAuthenticated { + let resetAction = CardPresentPaymentAction.reset + dispatch(resetAction) + } - sessionManager.reset() state = DeauthenticatedState() + sessionManager.reset() ServiceLocator.analytics.refreshUserData() ZendeskProvider.shared.reset()