Merge pull request #19754 from wordpress-mobile/feature/cmf-delete-exported-data-on-logout

Jetpack Migration: Clean up exported content on WP log out

Author: twstokes

WordPress/Classes/Services/BlogService.m

@@ -481,7 +481,7 @@ - (void)removeBlog:(Blog *)blog
         [accountService purgeAccountIfUnused:account];
     }
 
-    [[ContextManager sharedInstance] saveContext:self.managedObjectContext];
+    [[ContextManager sharedInstance] saveContextAndWait:self.managedObjectContext];
     [WPAnalytics refreshMetadata];
 }
 

WordPress/Classes/System/WordPressAppDelegate.swift

@@ -330,6 +330,9 @@ class WordPressAppDelegate: UIResponder, UIApplicationDelegate {
         setupWordPressExtensions()
 
         if FeatureFlag.contentMigration.enabled {
+            // To prevent race condition, initialize the shared instance synchronously so it can listen to account change notifications.
+            let _ = ContentMigrationCoordinator.shared
+
             // Start proactively exporting WP data in the background if the conditions are fulfilled.
             // This needs to be called after `setupWordPressExtensions` because it updates the stored data.
             DispatchQueue.global().async {

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

@@ -158,6 +158,17 @@ class BloggingRemindersScheduler {
         }
     }
 
+    /// Deletes backup reminders if it exists.
+    ///
+    static func deleteBackupReminders() {
+        guard FeatureFlag.contentMigration.enabled,
+              let sharedFileURL = sharedDataFileURL() else {
+            return
+        }
+
+        try? FileManager.default.removeItem(at: sharedFileURL)
+    }
+
     private static func copyStoreToSharedFile() {
         guard let store = try? defaultStore(),
               let sharedFileUrl = sharedDataFileURL() else {

WordPress/Classes/Utility/CoreDataHelper.swift

@@ -220,13 +220,12 @@ extension CoreDataStack {
         }
     }
 
+    // MARK: - Database Migration
+
     /// Creates a copy of the current open store and saves it to the specified destination
     /// - Parameter backupLocation: Location to save the store copy to
     func createStoreCopy(to backupLocation: URL) throws {
-        let (backupLocation, shmLocation, walLocation) = databaseFiles(for: backupLocation)
-        try? FileManager.default.removeItem(at: backupLocation)
-        try? FileManager.default.removeItem(at: shmLocation)
-        try? FileManager.default.removeItem(at: walLocation)
+        try? removeBackupData(from: backupLocation)
         guard let storeCoordinator = mainContext.persistentStoreCoordinator,
               let store = storeCoordinator.persistentStores.first else {
             throw ContextManager.ContextManagerError.missingCoordinatorOrStore
@@ -245,6 +244,15 @@ extension CoreDataStack {
                                                         withType: storeCopy.type)
     }
 
+    /// Removes any copy of the store from the backup location.
+    /// - Parameter backupLocation: Where the backup store is located.
+    func removeBackupData(from location: URL) throws {
+        let (backupLocation, shmLocation, walLocation) = databaseFiles(for: location)
+        try FileManager.default.removeItem(at: backupLocation)
+        try FileManager.default.removeItem(at: shmLocation)
+        try FileManager.default.removeItem(at: walLocation)
+    }
+
     /// Replaces the current active store with the database at the specified location.
     ///
     /// The following steps are performed:

WordPress/Classes/Utility/Migration/ContentMigrationCoordinator.swift

@@ -1,29 +1,37 @@
 /// Encapsulates logic related to content migration from WordPress to Jetpack.
 ///
-class ContentMigrationCoordinator {
+@objc class ContentMigrationCoordinator: NSObject {
 
-    static var shared: ContentMigrationCoordinator = {
+    @objc static var shared: ContentMigrationCoordinator = {
         .init()
     }()
 
     // MARK: Dependencies
 
     private let coreDataStack: CoreDataStack
     private let dataMigrator: ContentDataMigrating
+    private let notificationCenter: NotificationCenter
     private let userPersistentRepository: UserPersistentRepository
     private let eligibilityProvider: ContentMigrationEligibilityProvider
     private let tracker: MigrationAnalyticsTracker
 
     init(coreDataStack: CoreDataStack = ContextManager.shared,
          dataMigrator: ContentDataMigrating = DataMigrator(),
+         notificationCenter: NotificationCenter = .default,
          userPersistentRepository: UserPersistentRepository = UserDefaults.standard,
          eligibilityProvider: ContentMigrationEligibilityProvider = AppConfiguration(),
          tracker: MigrationAnalyticsTracker = .init()) {
         self.coreDataStack = coreDataStack
         self.dataMigrator = dataMigrator
+        self.notificationCenter = notificationCenter
         self.userPersistentRepository = userPersistentRepository
         self.eligibilityProvider = eligibilityProvider
         self.tracker = tracker
+
+        super.init()
+
+        // register for account change notification.
+        ensureBackupDataDeletedOnLogout()
     }
 
     enum ContentMigrationCoordinatorError: LocalizedError {
@@ -99,9 +107,25 @@ class ContentMigrationCoordinator {
             completion?()
         }
     }
+
+    /// Attempts to clean up the exported data by re-exporting user content if they're still eligible, or deleting them otherwise.
+    /// Re-exporting user content ensures that the exported data will match the latest state of Account and Blogs.
+    ///
+    @objc func cleanupExportedDataIfNeeded() {
+        // try to re-export the user content if they're still eligible.
+        startAndDo { [weak self] result in
+            switch result {
+            case .failure(let error) where error == .ineligible:
+                // if the user is no longer eligible, ensure that any exported contents are deleted.
+                self?.dataMigrator.deleteExportedData()
+            default:
+                break
+            }
+        }
+    }
 }
 
-// MARK: - Preflights Local Draft Check
+// MARK: - Private Helpers
 
 private extension ContentMigrationCoordinator {
 
@@ -119,11 +143,26 @@ private extension ContentMigrationCoordinator {
         return count == 0
     }
 
+    /// When the user logs out, ensure that any exported data is deleted if it exists at the backup location.
+    /// This prevents the user from entering the migration flow and immediately gets shown with a login pop-up (since we couldn't migrate the authToken anymore).
+    ///
+    func ensureBackupDataDeletedOnLogout() {
+        notificationCenter.addObserver(forName: .WPAccountDefaultWordPressComAccountChanged, object: nil, queue: nil) { [weak self] notification in
+            // nil notification object means it's a logout event.
+            guard let self,
+                  notification.object == nil else {
+                return
+            }
+
+            self.cleanupExportedDataIfNeeded()
+        }
+    }
 }
 
 // MARK: - Content Migration Eligibility Provider
 
 protocol ContentMigrationEligibilityProvider {
+    /// Determines if we should export user's content data in the current app state.
     var isEligibleForMigration: Bool { get }
 }
 

WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m

@@ -1699,6 +1699,11 @@ - (void)confirmRemoveSite
     BlogService *blogService = [[BlogService alloc] initWithManagedObjectContext:context];
     [blogService removeBlog:self.blog];
     [[WordPressAppDelegate shared] trackLogoutIfNeeded];
+
+    if ([Feature enabled:FeatureFlagContentMigration] && [AppConfiguration isWordPress]) {
+        [ContentMigrationCoordinator.shared cleanupExportedDataIfNeeded];
+    }
+
     [self.navigationController popToRootViewControllerAnimated:YES];
 }
 

WordPress/Classes/ViewRelated/Blog/Blog List/BlogListViewController.m

@@ -709,6 +709,11 @@ - (void)confirmRemoveSiteForIndexPath:(NSIndexPath *)indexPath
     NSManagedObjectContext *context = [[ContextManager sharedInstance] mainContext];
     BlogService *blogService = [[BlogService alloc] initWithManagedObjectContext:context];
     [blogService removeBlog:blog];
+
+    if ([Feature enabled:FeatureFlagContentMigration] && [AppConfiguration isWordPress]) {
+        [ContentMigrationCoordinator.shared cleanupExportedDataIfNeeded];
+    }
+
     [self.tableView reloadData];
 }
 

WordPress/Jetpack/Classes/Utility/DataMigrator.swift

@@ -1,6 +1,16 @@
 protocol ContentDataMigrating {
+    /// Exports user content data to a shared location that's accessible by the Jetpack app.
+    ///
+    /// - Parameter completion: Closure called after the export process completes.
     func exportData(completion: ((Result<Void, DataMigrationError>) -> Void)?)
+
+    /// Imports user's WordPress content data from the shared location.
+    ///
+    /// - Parameter completion: Closure called after the export process completes.
     func importData(completion: ((Result<Void, DataMigrationError>) -> Void)?)
+
+    /// Deletes any exported user content at the shared location if it exists.
+    func deleteExportedData()
 }
 
 enum DataMigrationError: LocalizedError {
@@ -83,6 +93,26 @@ extension DataMigrator: ContentDataMigrating {
         BloggingRemindersScheduler.handleRemindersMigration()
         completion?(.success(()))
     }
+
+    func deleteExportedData() {
+        guard let backupLocation,
+              let sharedDefaults else {
+            return
+        }
+
+        // mark this as false regardless if any of the steps below fails.
+        // this serves as the first stopgap that prevents the migration process on the Jetpack side.
+        isDataReadyToMigrate = false
+
+        // remove database backup
+        try? coreDataStack.removeBackupData(from: backupLocation)
+
+        // remove user defaults backup
+        sharedDefaults.removeObject(forKey: DefaultsWrapper.dictKey)
+
+        // remove blogging reminders backup
+        BloggingRemindersScheduler.deleteBackupReminders()
+    }
 }
 
 // MARK: - Private Functions

WordPress/WordPressTest/ContentMigrationCoordinatorTests.swift

@@ -8,6 +8,7 @@ final class ContentMigrationCoordinatorTests: CoreDataTestCase {
 
     private var mockEligibilityProvider: MockEligibilityProvider!
     private var mockDataMigrator: MockDataMigrator!
+    private var mockNotificationCenter: MockNotificationCenter!
     private var mockPersistentRepository: InMemoryUserDefaults!
     private var coordinator: ContentMigrationCoordinator!
 
@@ -16,13 +17,15 @@ final class ContentMigrationCoordinatorTests: CoreDataTestCase {
 
         mockEligibilityProvider = MockEligibilityProvider()
         mockDataMigrator = MockDataMigrator()
+        mockNotificationCenter = MockNotificationCenter()
         mockPersistentRepository = InMemoryUserDefaults()
         coordinator = makeCoordinator()
     }
 
     override func tearDown() {
         mockEligibilityProvider = nil
         mockDataMigrator = nil
+        mockNotificationCenter = nil
         mockPersistentRepository = nil
         coordinator = nil
 
@@ -181,6 +184,57 @@ final class ContentMigrationCoordinatorTests: CoreDataTestCase {
         wait(for: [expect], timeout: timeout)
     }
 
+    // MARK: Export data cleanup tests
+
+    func test_cleanupExportedData_givenUserIsIneligible_shouldDeleteData() {
+        // Given
+        mockEligibilityProvider.isEligibleForMigration = false
+
+        // When
+        coordinator.cleanupExportedDataIfNeeded()
+
+        // Then
+        XCTAssertTrue(mockDataMigrator.deleteExportedDataCalled)
+    }
+
+    func test_cleanupExportedData_givenUserIsEligible_shouldExportData() {
+        // When
+        coordinator.cleanupExportedDataIfNeeded()
+
+        // Then
+        XCTAssertTrue(mockDataMigrator.exportCalled)
+    }
+
+    func test_coordinatorShouldObserveLogoutNotifications() {
+        XCTAssertNotNil(mockNotificationCenter.observerBlock)
+        XCTAssertNotNil(mockNotificationCenter.observedNotificationName)
+        XCTAssertEqual(mockNotificationCenter.observedNotificationName, Foundation.Notification.Name.WPAccountDefaultWordPressComAccountChanged)
+    }
+
+    func test_givenLoginNotifications_coordinatorShouldDoNothing() {
+        // Given
+        let loginNotification = mockNotificationCenter.makeLoginNotification()
+
+        // When
+        mockNotificationCenter.observerBlock?(loginNotification)
+
+        // Then
+        XCTAssertFalse(mockDataMigrator.exportCalled)
+        XCTAssertFalse(mockDataMigrator.deleteExportedDataCalled)
+    }
+
+    func test_givenLogoutNotifications_coordinatorShouldPerformCleanup() {
+        // Given
+        mockEligibilityProvider.isEligibleForMigration = false
+        let logoutNotification = mockNotificationCenter.makeLogoutNotification()
+
+        // When
+        mockNotificationCenter.observerBlock?(logoutNotification)
+
+        // Then
+        XCTAssertFalse(mockDataMigrator.exportCalled)
+        XCTAssertTrue(mockDataMigrator.deleteExportedDataCalled)
+    }
 }
 
 // MARK: - Helpers
@@ -192,14 +246,15 @@ private extension ContentMigrationCoordinatorTests {
     }
 
     final class MockEligibilityProvider: ContentMigrationEligibilityProvider {
-        var isEligibleForMigration: Bool = true
+        var isEligibleForMigration = true
     }
 
     final class MockDataMigrator: ContentDataMigrating {
         var exportErrorToReturn: DataMigrationError? = nil
         var exportCalled = false
         var importErrorToReturn: DataMigrationError? = nil
         var importCalled = false
+        var deleteExportedDataCalled = false
 
         func exportData(completion: ((Result<Void, DataMigrationError>) -> Void)? = nil) {
             exportCalled = true
@@ -218,11 +273,16 @@ private extension ContentMigrationCoordinatorTests {
             }
             completion?(.failure(importErrorToReturn))
         }
+
+        func deleteExportedData() {
+            deleteExportedDataCalled = true
+        }
     }
 
     func makeCoordinator() -> ContentMigrationCoordinator {
         return .init(coreDataStack: contextManager,
                      dataMigrator: mockDataMigrator,
+                     notificationCenter: mockNotificationCenter,
                      userPersistentRepository: mockPersistentRepository,
                      eligibilityProvider: mockEligibilityProvider)
     }
@@ -235,3 +295,25 @@ private extension ContentMigrationCoordinatorTests {
     }
 
 }
+
+private final class MockNotificationCenter: NotificationCenter {
+    var observedNotificationName: NSNotification.Name? = nil
+    var observerBlock: ((Foundation.Notification) -> Void)? = nil
+
+    override func addObserver(forName name: NSNotification.Name?,
+                              object obj: Any?,
+                              queue: OperationQueue?,
+                              using block: @escaping @Sendable (Foundation.Notification) -> Void) -> NSObjectProtocol {
+        observedNotificationName = name
+        observerBlock = block
+        return NSNull()
+    }
+
+    func makeLoginNotification() -> Foundation.Notification {
+        return Foundation.Notification(name: .WPAccountDefaultWordPressComAccountChanged, object: String())
+    }
+
+    func makeLogoutNotification() -> Foundation.Notification {
+        return Foundation.Notification(name: .WPAccountDefaultWordPressComAccountChanged, object: nil)
+    }
+}