Work in progress: Fixing tests.

Signed-off-by: Iva Horn <iva.horn@nextcloud.com>

Author: i2h3

Sources/NextcloudFileProviderKit/Enumeration/RemoteChangeObserver.swift

@@ -9,13 +9,13 @@ import NextcloudKit
 
 public let NotifyPushAuthenticatedNotificationName = Notification.Name("NotifyPushAuthenticated")
 
-public actor RemoteChangeObserver: NSObject, Sendable {
+public final class RemoteChangeObserver: NSObject, @unchecked Sendable {
     // @unchecked Sendable is used because 'account' is mutable, but mutation is controlled and safe in this context.
     public let remoteInterface: RemoteInterface
     public let changeNotificationInterface: ChangeNotificationInterface
     public let domain: NSFileProviderDomain?
     public let dbManager: FilesDatabaseManager
-    private(set) var account: Account
+    public var account: Account
     public var accountId: String { account.ncKitAccount }
 
     public var webSocketPingIntervalNanoseconds: UInt64 = 3 * 1_000_000_000
@@ -41,14 +41,7 @@ public actor RemoteChangeObserver: NSObject, Sendable {
 
     private(set) var pollingTimer: Timer?
 
-    private(set) var pollInterval: TimeInterval = 60 {
-        didSet {
-            if pollingActive {
-                stopPollingTimer()
-                startPollingTimer()
-            }
-        }
-    }
+    let pollInterval: TimeInterval
 
     public var pollingActive: Bool {
         pollingTimer != nil
@@ -74,13 +67,15 @@ public actor RemoteChangeObserver: NSObject, Sendable {
         changeNotificationInterface: ChangeNotificationInterface,
         domain: NSFileProviderDomain?,
         dbManager: FilesDatabaseManager,
+        pollInterval: TimeInterval = 60,
         log: any FileProviderLogging
     ) {
         self.account = account
         self.remoteInterface = remoteInterface
         self.changeNotificationInterface = changeNotificationInterface
         self.domain = domain
         self.dbManager = dbManager
+        self.pollInterval = pollInterval
         logger = FileProviderLogger(category: "RemoteChangeObserver", log: log)
         super.init()
 
@@ -95,17 +90,20 @@ public actor RemoteChangeObserver: NSObject, Sendable {
     }
 
     private func startPollingTimer() {
-        guard !invalidated else { return }
+        guard !invalidated else {
+            self.logger.error("Starting polling timer while the current one is not invalidated yet!")
+            return
+        }
+
         Task {
-            pollingTimer = Timer.scheduledTimer(
-                withTimeInterval: pollInterval, repeats: true
-            ) { [weak self] _ in
+            pollingTimer = Timer.scheduledTimer(withTimeInterval: pollInterval, repeats: true) { [weak self] _ in
                 self?.logger.info("Polling timer timeout, notifying change.")
 
                 Task {
                     await self?.startWorkingSetCheck()
                 }
             }
+
             logger.info("Starting polling timer.")
         }
     }
@@ -119,11 +117,13 @@ public actor RemoteChangeObserver: NSObject, Sendable {
     }
 
     public func invalidate() {
+        logger.debug("Invalidating.")
         invalidated = true
         resetWebSocket()
     }
 
     private func reconnectWebSocket() {
+        logger.debug("Reconnecting web socket...")
         stopPollingTimer()
         resetWebSocket()
 
@@ -145,6 +145,7 @@ public actor RemoteChangeObserver: NSObject, Sendable {
     }
 
     public func resetWebSocket() {
+        logger.debug("Resetting web socket...")
         webSocketTask?.cancel()
         webSocketUrlSession = nil
         webSocketTask = nil
@@ -156,7 +157,10 @@ public actor RemoteChangeObserver: NSObject, Sendable {
     }
 
     private func configureNotifyPush() async {
+        logger.debug("Configuring notify push...")
+
         guard !invalidated else {
+            logger.error("Attempt to configure notify push while being invalidated!")
             return
         }
 
@@ -369,10 +373,6 @@ public actor RemoteChangeObserver: NSObject, Sendable {
         self.account = account
     }
 
-    func setPollInterval(to interval: TimeInterval) {
-        self.pollInterval = interval
-    }
-
     func setWebSocketPingInterval(to nanoseconds: UInt64) {
         self.webSocketPingIntervalNanoseconds = nanoseconds
     }

Tests/Interface/MockChangeNotificationInterface.swift

@@ -4,10 +4,10 @@
 import Foundation
 import NextcloudFileProviderKit
 
-public actor MockChangeNotificationInterface: @preconcurrency ChangeNotificationInterface {
+public final class MockChangeNotificationInterface:  ChangeNotificationInterface {
     public typealias ChangeHandler = @Sendable () -> Void
 
-    private(set) var changeHandler: ChangeHandler?
+    let changeHandler: ChangeHandler?
 
     public init(changeHandler: ChangeHandler? = nil) {
         self.changeHandler = changeHandler
@@ -16,8 +16,4 @@ public actor MockChangeNotificationInterface: @preconcurrency ChangeNotification
     public func notifyChange() {
         changeHandler?()
     }
-
-    public func replaceChangeHandler(with newHandler: ChangeHandler?) {
-        self.changeHandler = newHandler
-    }
 }

Tests/NextcloudFileProviderKitTests/ExpectationFulfillmentCounter.swift

@@ -0,0 +1,30 @@
+import XCTest
+
+///
+/// A concurrency-safe counter for the sequential fulfillment of multiple expectations.
+///
+actor ExpectationFulfillmentCounter {
+    ///
+    /// The number of increments during the lifetime of this object.
+    ///
+    private(set) var count = 0
+
+    let expectations: [XCTestExpectation]
+
+    init(_ expectations: XCTestExpectation...) {
+        self.expectations = expectations
+    }
+
+    ///
+    /// Increase the state by one.
+    ///
+    func next(file: StaticString = #filePath, line: UInt = #line) {
+        guard expectations.count > count else {
+            XCTFail("Insufficient expectations to fulfill!", file: file, line: line)
+            return
+        }
+
+        expectations[count].fulfill()
+        count += 1
+    }
+}

Tests/NextcloudFileProviderKitTests/RemoteChangeObserverTests.swift

@@ -83,6 +83,7 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         Self.notifyPushServer.delay = 1_000_000
 
         let authenticated = expectation(description: "authenticated")
+        authenticated.assertForOverFulfill = false
 
         NotificationCenter.default.addObserver(forName: NotifyPushAuthenticatedNotificationName, object: nil, queue: nil) { _ in
             authenticated.fulfill()
@@ -156,38 +157,33 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         debugPrint(db)
 
         let testStartDate = Date()
-        let remoteInterface =
-            MockRemoteInterface(account: Self.account, rootItem: MockRemoteItem.rootItem(account: Self.account))
+        let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: MockRemoteItem.rootItem(account: Self.account))
         remoteInterface.capabilities = mockCapabilities
 
         // --- DB State (What the app thinks is true) ---
         // A materialized file in the root that will be updated.
-        var rootFileToUpdate =
-            SendableItemMetadata(ocId: "rootFile", fileName: "root-file.txt", account: Self.account)
+        var rootFileToUpdate = SendableItemMetadata(ocId: "rootFile", fileName: "root-file.txt", account: Self.account)
         rootFileToUpdate.downloaded = true
         rootFileToUpdate.etag = "ETAG_OLD_ROOTFILE"
         Self.dbManager.addItemMetadata(rootFileToUpdate)
 
         // A materialized folder that will have its contents changed.
-        var folderA =
-            SendableItemMetadata(ocId: "folderA", fileName: "FolderA", account: Self.account)
+        var folderA = SendableItemMetadata(ocId: "folderA", fileName: "FolderA", account: Self.account)
         folderA.directory = true
         folderA.visitedDirectory = true
         folderA.etag = "ETAG_OLD_FOLDERA"
         Self.dbManager.addItemMetadata(folderA)
 
         // A materialized file inside FolderA that will be deleted.
-        var fileInAToDelete =
-            SendableItemMetadata(ocId: "fileInA", fileName: "file-in-a.txt", account: Self.account)
+        var fileInAToDelete = SendableItemMetadata(ocId: "fileInA", fileName: "file-in-a.txt", account: Self.account)
         fileInAToDelete.downloaded = true
         fileInAToDelete.serverUrl = Self.account.davFilesUrl + "/FolderA"
         // Set an explicit old sync time to verify it gets updated during deletion
         fileInAToDelete.syncTime = Date(timeIntervalSince1970: 1000)
         Self.dbManager.addItemMetadata(fileInAToDelete)
 
         // A materialized folder that will be deleted entirely.
-        var folderBToDelete =
-            SendableItemMetadata(ocId: "folderB", fileName: "FolderB", account: Self.account)
+        var folderBToDelete = SendableItemMetadata(ocId: "folderB", fileName: "FolderB", account: Self.account)
         folderBToDelete.directory = true
         folderBToDelete.visitedDirectory = true
         // Set an explicit old sync time to verify it gets updated during deletion
@@ -243,9 +239,8 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         let authExpectation =
             XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName)
         let changeNotifiedExpectation = XCTestExpectation(description: "Change Notified")
-        let notificationInterface = MockChangeNotificationInterface()
 
-        await notificationInterface.replaceChangeHandler {
+        let notificationInterface = MockChangeNotificationInterface {
             changeNotifiedExpectation.fulfill()
         }
 
@@ -303,16 +298,14 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         remoteInterface.capabilities = mockCapabilities
 
         let authenticated = expectation(description: "authenticated")
-        let notified = expectation(description: "notified")
+        authenticated.assertForOverFulfill = false
 
         NotificationCenter.default.addObserver(forName: NotifyPushAuthenticatedNotificationName, object: nil, queue: nil) { _ in
             authenticated.fulfill()
         }
 
-        let notificationInterface = MockChangeNotificationInterface()
-
-        await notificationInterface.replaceChangeHandler {
-            notified.fulfill()
+        let notificationInterface = MockChangeNotificationInterface {
+            XCTFail("This notification should not happen!")
         }
 
         remoteChangeObserver = RemoteChangeObserver(
@@ -329,8 +322,6 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         Self.notifyPushServer.send(message: "random")
         Self.notifyPushServer.send(message: "notify_activity")
         Self.notifyPushServer.send(message: "notify_notification")
-
-        await fulfillment(of: [notified])
     }
 
     func testPolling() async throws {
@@ -353,9 +344,8 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         remoteInterface.rootItem?.children = [serverItem]
 
         let changeNotifiedExpectation = XCTestExpectation(description: "Change Notified via Polling")
-        let notificationInterface = MockChangeNotificationInterface()
 
-        await notificationInterface.replaceChangeHandler {
+        let notificationInterface = MockChangeNotificationInterface {
             changeNotifiedExpectation.fulfill()
         }
 
@@ -365,15 +355,15 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
             changeNotificationInterface: notificationInterface,
             domain: nil,
             dbManager: Self.dbManager,
+            pollInterval: 0.5,
             log: FileProviderLogMock()
         )
-        // Set a very short poll interval for the test.
-        await remoteChangeObserver?.setPollInterval(to: 0.5)
 
         // 2. Act & Assert
         // The observer will fail to connect to websocket and start polling.
         // We just need to wait for the poll to fire and detect the change.
         await wait(for: changeNotifiedExpectation, description: "polling to trigger change")
+
         let pollingActive = await remoteChangeObserver?.pollingActive ?? false
         XCTAssertTrue(pollingActive, "Polling should be active.")
     }
@@ -384,26 +374,13 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
 
         let authenticated = expectation(description: "authenticated")
         let reauthenticated = expectation(description: "reauthenticated")
-
-        actor AuthCounter {
-            var count = 0
-            func increment() -> Int {
-                count += 1
-                return count
-            }
-        }
-        let authCounter = AuthCounter()
+        let fulfillments = ExpectationFulfillmentCounter(authenticated, reauthenticated)
 
         NotificationCenter.default.addObserver(
             forName: NotifyPushAuthenticatedNotificationName, object: nil, queue: nil
         ) { _ in
             Task {
-                let count = await authCounter.increment()
-                if count == 1 {
-                    authenticated.fulfill()
-                } else if count == 2 {
-                    reauthenticated.fulfill()
-                }
+                await fulfillments.next()
             }
         }
 
@@ -508,7 +485,17 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         )
         remoteInterface.rootItem?.children = [serverItem]
 
-        let notificationInterface = MockChangeNotificationInterface()
+        let change1 = XCTestExpectation(description: "First change notification")
+        let change2 = XCTestExpectation(description: "Second change notification")
+
+        let fulfillments = ExpectationFulfillmentCounter(change1, change2)
+
+        let notificationInterface = MockChangeNotificationInterface {
+            Task {
+                await fulfillments.next()
+            }
+        }
+
         let remoteChangeObserver = RemoteChangeObserver(
             account: Self.account,
             remoteInterface: remoteInterface,
@@ -520,19 +507,12 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         self.remoteChangeObserver = remoteChangeObserver
 
         // --- Phase 1: Test connection and change notification ---
-        let authExpectation =
-            XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName)
+        let authExpectation = XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName)
         remoteChangeObserver.networkReachabilityObserver(.reachableEthernetOrWiFi)
         await wait(for: authExpectation, description: "initial authentication")
 
-        let changeExpectation1 = XCTestExpectation(description: "First change notification")
-
-        await notificationInterface.replaceChangeHandler {
-            changeExpectation1.fulfill()
-        }
-
         Self.notifyPushServer.send(message: "notify_file")
-        await wait(for: changeExpectation1, description: "first change")
+        await wait(for: change1, description: "first change")
 
         // --- Phase 2: Test connection loss ---
         remoteChangeObserver.networkReachabilityObserver(.notReachable)
@@ -544,8 +524,7 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
         Self.notifyPushServer.reset()
 
         // --- Phase 3: Test reconnection and change notification ---
-        let reauthExpectation =
-            XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName)
+        let reauthExpectation = XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName)
 
         // Trigger the reconnection logic.
         remoteChangeObserver.networkReachabilityObserver(.reachableEthernetOrWiFi)
@@ -556,13 +535,7 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase {
 
         XCTAssertTrue(webSocketTaskActiveAfterReconnect, "Websocket should be active again after reconnection.")
 
-        let changeExpectation2 = XCTestExpectation(description: "Second change notification")
-
-        await notificationInterface.replaceChangeHandler {
-            changeExpectation2.fulfill()
-        }
-
         Self.notifyPushServer.send(message: "notify_file")
-        await wait(for: changeExpectation2, description: "second change")
+        await wait(for: change2, description: "second change")
     }
 }