Add function to stop ongoing syncs when we log out

Author: joshheald

Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift

@@ -641,6 +641,10 @@ final class POSPreviewCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol
     func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
         return false
     }
+
+    func stopOngoingSyncs(for siteID: Int64) async {
+        // Preview implementation - no-op
+    }
 }
 
 #endif

Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift

@@ -42,6 +42,10 @@ public protocol POSCatalogSyncCoordinatorProtocol {
     ///   - maxDays: Maximum number of days before a sync is considered stale
     /// - Returns: True if the last sync is older than the specified days or if there has been no sync
     func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool
+
+    /// Stops all ongoing sync tasks for the specified site
+    /// - Parameter siteID: The site ID to stop syncs for
+    func stopOngoingSyncs(for siteID: Int64) async
 }
 
 public extension POSCatalogSyncCoordinatorProtocol {
@@ -81,6 +85,12 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
     /// Tracks ongoing incremental syncs by site ID to prevent duplicates
     private var ongoingIncrementalSyncs: Set<Int64> = []
 
+    /// Tracks ongoing full sync tasks by site ID for cancellation
+    private var ongoingFullSyncTasks: [Int64: Task<Void, Error>] = [:]
+
+    /// Tracks ongoing incremental sync tasks by site ID for cancellation
+    private var ongoingIncrementalSyncTasks: [Int64: Task<Void, Error>] = [:]
+
     /// Observable model for full sync state updates
     public nonisolated let fullSyncStateModel: POSCatalogSyncStateModel = .init()
 
@@ -117,10 +127,22 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
         let allowCellular = isFirstSync || siteSettings.getPOSLocalCatalogCellularDataAllowed(siteID: siteID)
         DDLogInfo("🔄 POSCatalogSyncCoordinator starting full sync for site \(siteID)")
 
-        do {
+        // Create a task to perform the sync
+        let syncTask = Task<Void, Error> {
             _ = try await fullSyncService.startFullSync(for: siteID,
                                                         regenerateCatalog: regenerateCatalog,
                                                         allowCellular: allowCellular)
+        }
+
+        // Store the task for potential cancellation
+        ongoingFullSyncTasks[siteID] = syncTask
+
+        defer {
+            ongoingFullSyncTasks.removeValue(forKey: siteID)
+        }
+
+        do {
+            try await syncTask.value
             emitSyncState(.syncCompleted(siteID: siteID))
         } catch AFError.explicitlyCancelled, is CancellationError {
             if isFirstSync {
@@ -245,10 +267,22 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
 
         DDLogInfo("🔄 POSCatalogSyncCoordinator starting incremental sync for site \(siteID)")
 
-        do {
+        // Create a task to perform the sync
+        let syncTask = Task<Void, Error> {
             try await incrementalSyncService.startIncrementalSync(for: siteID,
                                                                   lastFullSyncDate: lastFullSyncDate,
-                                                                  lastIncrementalSyncDate: lastIncrementalSyncDate(for: siteID))
+                                                                  lastIncrementalSyncDate: await lastIncrementalSyncDate(for: siteID))
+        }
+
+        // Store the task for potential cancellation
+        ongoingIncrementalSyncTasks[siteID] = syncTask
+
+        defer {
+            ongoingIncrementalSyncTasks.removeValue(forKey: siteID)
+        }
+
+        do {
+            try await syncTask.value
         } catch AFError.explicitlyCancelled, is CancellationError {
             throw POSCatalogSyncError.requestCancelled
         }
@@ -349,6 +383,42 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
 
         return lastFullSync < thresholdDate
     }
+
+    public func stopOngoingSyncs(for siteID: Int64) async {
+        DDLogInfo("🛑 POSCatalogSyncCoordinator: Stopping ongoing syncs for site \(siteID)")
+
+        // Cancel ongoing full sync task if exists
+        if let fullSyncTask = ongoingFullSyncTasks[siteID] {
+            fullSyncTask.cancel()
+            ongoingFullSyncTasks.removeValue(forKey: siteID)
+            DDLogInfo("🛑 POSCatalogSyncCoordinator: Cancelled full sync task for site \(siteID)")
+        }
+
+        // Cancel ongoing incremental sync task if exists
+        if let incrementalSyncTask = ongoingIncrementalSyncTasks[siteID] {
+            incrementalSyncTask.cancel()
+            ongoingIncrementalSyncTasks.removeValue(forKey: siteID)
+            DDLogInfo("🛑 POSCatalogSyncCoordinator: Cancelled incremental sync task for site \(siteID)")
+        }
+
+        // Clean up incremental sync tracking
+        if ongoingIncrementalSyncs.contains(siteID) {
+            ongoingIncrementalSyncs.remove(siteID)
+            DDLogInfo("🛑 POSCatalogSyncCoordinator: Cleaned up incremental sync tracking for site \(siteID)")
+        }
+
+        // Update sync state to reflect that syncs are being stopped
+        // This will prevent new syncs from starting for this site
+        if let currentState = fullSyncStateModel.state[siteID] {
+            switch currentState {
+            case .initialSyncStarted, .syncStarted:
+                emitSyncState(.syncFailed(siteID: siteID, error: POSCatalogSyncError.requestCancelled))
+                DDLogInfo("🛑 POSCatalogSyncCoordinator: Updated sync state to cancelled for site \(siteID)")
+            default:
+                break
+            }
+        }
+    }
 }
 
 // MARK: - Syncing State

Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift

@@ -74,4 +74,6 @@ final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
     func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
         return isSyncStaleResult
     }
+
+    func stopOngoingSyncs(for siteID: Int64) async {}
 }

Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift

@@ -930,6 +930,125 @@ extension POSCatalogSyncCoordinatorTests {
         #expect(isStale == true)
     }
 
+    // MARK: - Stop Ongoing Syncs Tests
+
+    @Test func stopOngoingSyncs_clears_incremental_sync_tracking() async throws {
+        // Given - start an incremental sync
+        let fullSyncDate = Date().addingTimeInterval(-3600)
+        try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate)
+        mockIncrementalSyncService.blockNextSync()
+
+        let syncTask = Task {
+            try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge)
+        }
+
+        // Give sync a moment to start
+        try await Task.sleep(nanoseconds: 10_000_000) // 10ms
+
+        // When - stop ongoing syncs
+        await sut.stopOngoingSyncs(for: sampleSiteID)
+
+        // Then - incremental sync tracking should be cleared
+        // Attempting to start another sync should succeed (not throw syncAlreadyInProgress)
+        mockIncrementalSyncService.resumeBlockedSync()
+        _ = try? await syncTask.value
+
+        mockIncrementalSyncService.startIncrementalSyncResult = .success(())
+        try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge)
+        #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 2)
+    }
+
+    @Test func stopOngoingSyncs_updates_full_sync_state_when_sync_in_progress() async throws {
+        // Given - start a full sync
+        mockSyncService.blockNextSync()
+        mockSyncService.startFullSyncResult = .success(POSCatalog(products: [], variations: [], syncDate: .now))
+
+        let syncTask = Task {
+            try await sut.performFullSync(for: sampleSiteID)
+        }
+
+        // Give sync a moment to start
+        try await Task.sleep(nanoseconds: 10_000_000) // 10ms
+
+        // Verify sync is in progress
+        let stateBeforeStop = await sut.loadLastFullSyncState(for: sampleSiteID)
+        let isSyncInProgress: Bool = switch stateBeforeStop {
+        case .initialSyncStarted, .syncStarted: true
+        default: false
+        }
+        #expect(isSyncInProgress)
+
+        // When - stop ongoing syncs
+        await sut.stopOngoingSyncs(for: sampleSiteID)
+
+        // Then - sync state should be updated to failed with requestCancelled
+        let stateAfterStop = await sut.loadLastFullSyncState(for: sampleSiteID)
+        let isFailed: Bool = switch stateAfterStop {
+        case .syncFailed(let siteID, let error):
+            siteID == sampleSiteID && (error as? POSCatalogSyncError) == .requestCancelled
+        case .initialSyncFailed(let siteID, let error):
+            siteID == sampleSiteID && (error as? POSCatalogSyncError) == .requestCancelled
+        default: false
+        }
+        #expect(isFailed)
+
+        // Cleanup
+        mockSyncService.resumeBlockedSync()
+        _ = try? await syncTask.value
+    }
+
+    @Test func stopOngoingSyncs_does_nothing_when_no_sync_in_progress() async throws {
+        // Given - no sync in progress
+        try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: Date().addingTimeInterval(-3600))
+
+        let stateBeforeStop = await sut.loadLastFullSyncState(for: sampleSiteID)
+
+        // When - stop ongoing syncs
+        await sut.stopOngoingSyncs(for: sampleSiteID)
+
+        // Then - state should remain unchanged
+        let stateAfterStop = await sut.loadLastFullSyncState(for: sampleSiteID)
+        #expect(stateBeforeStop == stateAfterStop)
+    }
+
+    @Test func stopOngoingSyncs_handles_different_sites_independently() async throws {
+        // Given - syncs for two different sites
+        let siteA: Int64 = 123
+        let siteB: Int64 = 456
+        let fullSyncDate = Date().addingTimeInterval(-3600)
+
+        try createSiteInDatabase(siteID: siteA, lastFullSyncDate: fullSyncDate)
+        try createSiteInDatabase(siteID: siteB, lastFullSyncDate: fullSyncDate)
+
+        mockIncrementalSyncService.blockNextSync()
+
+        // Start syncs for both sites
+        let syncTaskA = Task {
+            try await sut.performIncrementalSyncIfApplicable(for: siteA, maxAge: sampleMaxAge)
+        }
+        let syncTaskB = Task {
+            try await sut.performIncrementalSyncIfApplicable(for: siteB, maxAge: sampleMaxAge)
+        }
+
+        try await Task.sleep(nanoseconds: 10_000_000) // 10ms
+
+        // When - stop syncs only for siteA
+        await sut.stopOngoingSyncs(for: siteA)
+
+        // Then - siteB sync should still throw syncAlreadyInProgress
+        await #expect(throws: POSCatalogSyncError.syncAlreadyInProgress(siteID: siteB)) {
+            try await sut.performIncrementalSyncIfApplicable(for: siteB, maxAge: sampleMaxAge)
+        }
+
+        // But siteA should allow new sync
+        mockIncrementalSyncService.resumeBlockedSync()
+        _ = try? await syncTaskA.value
+        _ = try? await syncTaskB.value
+
+        mockIncrementalSyncService.startIncrementalSyncResult = .success(())
+        try await sut.performIncrementalSyncIfApplicable(for: siteA, maxAge: sampleMaxAge)
+    }
+
     // MARK: - Cellular Data Tests
 
     @Test func performFullSync_allows_cellular_for_first_sync() async throws {

WooCommerce/Classes/Yosemite/DefaultStoresManager.swift

@@ -300,6 +300,13 @@ class DefaultStoresManager: StoresManager {
             dispatch(resetAction)
         }
 
+        // Stop any ongoing catalog sync tasks before resetting session
+        if let siteID = sessionManager.defaultStoreID {
+            Task {
+                await posCatalogSyncCoordinator?.stopOngoingSyncs(for: siteID)
+            }
+        }
+
         sessionManager.deleteApplicationPassword(locally: true)
         sessionManager.reset()
         state = DeauthenticatedState()

WooCommerce/WooCommerceTests/Tools/ForegroundPOSCatalogSyncDispatcherTests.swift

@@ -301,4 +301,6 @@ private final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProt
     func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
         return false
     }
+
+    func stopOngoingSyncs(for siteID: Int64) async {}
 }

WooCommerce/WooCommerceTests/Yosemite/StoresManagerTests.swift

@@ -157,6 +157,30 @@ final class StoresManagerTests: XCTestCase {
         XCTAssertTrue(cardPresentPaymentOnboardingStateCache.invalidateCalled)
     }
 
+    /// Verifies that `deauthenticate` handles catalog sync cleanup gracefully when there is a default store.
+    ///
+    @MainActor
+    func testDeauthenticate_handles_catalog_sync_cleanup_with_default_store() async {
+        // Arrange
+        let sessionManager = SessionManager.testingInstance
+        let manager = DefaultStoresManager(sessionManager: sessionManager,
+                                           notificationCenter: MockNotificationCenter.testingInstance)
+        manager.authenticate(credentials: SessionSettings.wpcomCredentials)
+        manager.updateDefaultStore(storeID: 123)
+
+        XCTAssertEqual(sessionManager.defaultStoreID, 123, "Store ID should be set before deauthentication")
+
+        // Action - should not crash even with default store ID set
+        manager.deauthenticate()
+
+        // Give any async cleanup time to execute
+        try? await Task.sleep(nanoseconds: 100_000_000) // 0.1 seconds
+
+        // Assert - verify deauthentication completed successfully
+        XCTAssertFalse(manager.isAuthenticated, "Manager should be deauthenticated")
+        XCTAssertNil(sessionManager.defaultStoreID, "Default store ID should be cleared after deauthentication")
+    }
+
     /// Verifies that `authenticate(username: authToken:)` persists the Credentials in the Keychain Storage.
     ///
     func testAuthenticatePersistsDefaultCredentialsInKeychain() {