Merge pull request #10010 from nextcloud/backport/10009/stable-33.0

[stable-33.0] Apply eager content policy also to first item in nested hierarchies

Author: i2h3

shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift

@@ -447,6 +447,52 @@ public final class FilesDatabaseManager: Sendable {
         }
     }
 
+    ///
+    /// Add or replace `metadata` while carrying over local-only fields the
+    /// server payload cannot know about: ``keepDownloaded``, ``downloaded``,
+    /// ``visitedDirectory``, and ``lockToken``.
+    ///
+    /// Mirrors the preservation set applied by
+    /// ``processItemMetadatasToUpdate`` for non-paginated reads. Use this from
+    /// any code path that ingests fresh PROPFIND results (e.g. paginated
+    /// enumeration); plain ``addItemMetadata(_:)`` would otherwise overwrite
+    /// these fields back to their defaults via Realm's `update: .all`,
+    /// silently undoing user-visible state such as "Always keep downloaded"
+    /// (#9923).
+    ///
+    /// Returns the merged metadata that was persisted. Callers that report
+    /// items back to the file-provider framework MUST forward the returned
+    /// value rather than the input — otherwise the framework receives the
+    /// pre-merge defaults and renders the item as if the local-only state
+    /// (e.g. pinned-via-keep-downloaded) had been cleared.
+    ///
+    /// - Parameters:
+    ///   - metadata: The freshly-built metadata to persist.
+    ///   - preserveVisitedDirectory: When `false`, do not carry over
+    ///     ``visitedDirectory`` from the existing row. Callers that have just
+    ///     visited the directory in the current request should pass `false`
+    ///     and pre-set `metadata.visitedDirectory = true`, so the visit is
+    ///     recorded rather than overwritten by a stale `false` from the DB.
+    ///
+    @discardableResult
+    public func addItemMetadataPreservingLocalState(_ metadata: SendableItemMetadata, preserveVisitedDirectory: Bool = true) -> SendableItemMetadata {
+        var toWrite = metadata
+
+        if let existing = itemMetadatas.where({ $0.ocId == metadata.ocId }).first {
+            toWrite.downloaded = existing.downloaded
+            toWrite.keepDownloaded = existing.keepDownloaded
+
+            if preserveVisitedDirectory {
+                toWrite.visitedDirectory = existing.visitedDirectory
+            }
+
+            toWrite.lockToken = existing.lockToken
+        }
+
+        addItemMetadata(toWrite)
+        return toWrite
+    }
+
     ///
     /// Mark an item as deleted.
     ///

shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift

@@ -15,17 +15,35 @@ extension Enumerator {
         if pageIndex == 0 {
             guard let firstFile = files.first else { return (nil, .invalidResponseError) }
             var metadata = firstFile.toItemMetadata()
+
             if metadata.directory {
+                // Mark the directory as visited in this request, then persist
+                // while preserving the rest of the local-only state. Skip
+                // preservation of `visitedDirectory` so the just-set `true`
+                // is not overwritten by the existing row's value.
                 metadata.visitedDirectory = true
-                if let existingMetadata = dbManager.itemMetadata(ocId: metadata.ocId) {
-                    metadata.downloaded = existingMetadata.downloaded
-                    metadata.keepDownloaded = existingMetadata.keepDownloaded
-                }
+                dbManager.addItemMetadataPreservingLocalState(metadata, preserveVisitedDirectory: false)
+            } else {
+                dbManager.addItemMetadataPreservingLocalState(metadata)
             }
-            dbManager.addItemMetadata(metadata)
         }
-        let metadatas = files[startIndex...].map { $0.toItemMetadata() }
-        metadatas.forEach { dbManager.addItemMetadata($0) }
+        
+        // Persist children — including those on follow-up pages — while
+        // carrying over any local-only state previously set on existing rows
+        // (e.g. `keepDownloaded` from a recursive "Always keep downloaded"
+        // enable). A plain `addItemMetadata` would clobber those flags via
+        // Realm's `update: .all`, which is the root cause of #9923.
+        //
+        // Use the merged metadata returned by the helper for both the DB
+        // write and the value reported back to the framework. Returning the
+        // pre-merge fresh-from-server metadata would tell the framework
+        // `keepDownloaded == false` for items that are pinned in the
+        // database, leaving the OS view (`isKeepDownloaded`, `contentPolicy`)
+        // out of sync with the local truth.
+        let metadatas = files[startIndex...].map { file -> SendableItemMetadata in
+            dbManager.addItemMetadataPreservingLocalState(file.toItemMetadata())
+        }
+
         return (metadatas, nil)
     }
 

shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift

@@ -472,6 +472,145 @@ final class EnumeratorTests: NextcloudFileProviderKitTestCase {
         )
     }
 
+    ///
+    /// Children persisted by a page-0 paginated read must keep their existing
+    /// ``keepDownloaded`` flag. Before the fix for #9923 the bulk write at the
+    /// end of ``Enumerator.handlePagedReadResults`` used ``addItemMetadata``
+    /// which replaces the entire row, silently clearing flags set by a prior
+    /// "Always keep downloaded" enable.
+    ///
+    /// The metadatas returned to the enumeration observer must also reflect
+    /// the preserved state: the framework builds `Item.contentPolicy` from
+    /// `metadata.keepDownloaded` on whatever metadata we hand it, so reporting
+    /// the pre-merge fresh-from-server metadata would leave the OS view of
+    /// pinned descendants stuck on `.inherited` even though the database row
+    /// is correct. (Symptom observed in #9923 for `/Documents/Documenter`.)
+    ///
+    func testHandlePagedReadResultsPreservesKeepDownloadedOnPageZeroChildren() {
+        let dbManager = Self.dbManager
+        debugPrint(dbManager.ncDatabase())
+
+        // Seed two of the upcoming page-0 children as already pinned. The
+        // third stays unseeded so we can also assert the flag does not leak
+        // onto items that never had it.
+        let pinnedChildren = ["pagedChild0", "pagedChild2"]
+        for ocId in pinnedChildren {
+            var seeded = SendableItemMetadata(ocId: ocId, fileName: "\(ocId).txt", account: Self.account)
+            seeded.keepDownloaded = true
+            dbManager.addItemMetadata(seeded)
+        }
+
+        let parentNKFile = remoteFolder.toNKFile()
+        let childrenNKFiles = (0 ..< 3).map { i in
+            MockRemoteItem(
+                identifier: "pagedChild\(i)",
+                name: "pagedChild\(i).txt",
+                remotePath: Self.account.davFilesUrl + "/folder/pagedChild\(i).txt",
+                account: Self.account.ncKitAccount,
+                username: Self.account.username,
+                userId: Self.account.id,
+                serverUrl: Self.account.serverUrl
+            ).toNKFile()
+        }
+
+        let (returnedMetadatas, error) = Enumerator.handlePagedReadResults(
+            files: [parentNKFile] + childrenNKFiles, pageIndex: 0, dbManager: dbManager
+        )
+        XCTAssertNil(error)
+
+        for ocId in pinnedChildren {
+            let stored = dbManager.itemMetadata(ocId: ocId)
+            XCTAssertEqual(
+                stored?.keepDownloaded, true,
+                "Page-0 child \(ocId) must retain keepDownloaded across the paginated write (#9923)."
+            )
+            let returned = returnedMetadatas?.first(where: { $0.ocId == ocId })
+            XCTAssertEqual(
+                returned?.keepDownloaded, true,
+                "Page-0 child \(ocId) reported back to the enumeration observer must also reflect the preserved keepDownloaded — the framework builds contentPolicy from this value."
+            )
+        }
+        XCTAssertEqual(
+            dbManager.itemMetadata(ocId: "pagedChild1")?.keepDownloaded, false,
+            "Children that were never pinned must not gain the flag from the paginated write."
+        )
+    }
+
+    ///
+    /// Same protection on follow-up pages: a paginated read with `pageIndex > 0`
+    /// must not clobber ``keepDownloaded`` on any of its items. This is the
+    /// path most likely to land on already-pinned descendants because the
+    /// recursive enable of "Always keep downloaded" runs without pagination
+    /// while the OS-driven enumeration that follows uses pagination.
+    ///
+    func testHandlePagedReadResultsPreservesKeepDownloadedOnFollowUpPage() {
+        let dbManager = Self.dbManager
+        debugPrint(dbManager.ncDatabase())
+
+        var seeded = SendableItemMetadata(ocId: "pagedChild5", fileName: "pagedChild5.txt", account: Self.account)
+        seeded.keepDownloaded = true
+        dbManager.addItemMetadata(seeded)
+
+        let followUpChildrenNKFiles = (5 ..< 8).map { i in
+            MockRemoteItem(
+                identifier: "pagedChild\(i)",
+                name: "pagedChild\(i).txt",
+                remotePath: Self.account.davFilesUrl + "/folder/pagedChild\(i).txt",
+                account: Self.account.ncKitAccount,
+                username: Self.account.username,
+                userId: Self.account.id,
+                serverUrl: Self.account.serverUrl
+            ).toNKFile()
+        }
+
+        let (returnedMetadatas, error) = Enumerator.handlePagedReadResults(
+            files: followUpChildrenNKFiles, pageIndex: 1, dbManager: dbManager
+        )
+        XCTAssertNil(error)
+
+        XCTAssertEqual(
+            dbManager.itemMetadata(ocId: "pagedChild5")?.keepDownloaded, true,
+            "Follow-up page child must retain keepDownloaded across the paginated write (#9923)."
+        )
+        XCTAssertEqual(
+            returnedMetadatas?.first(where: { $0.ocId == "pagedChild5" })?.keepDownloaded, true,
+            "Follow-up page child reported back to the enumeration observer must also reflect the preserved keepDownloaded."
+        )
+    }
+
+    ///
+    /// The page-0 target directory must continue to be marked as visited
+    /// after the preservation refactor — the original inline path applied
+    /// `visitedDirectory = true` unconditionally, and that behaviour must
+    /// survive even when a previous DB row carries `visitedDirectory = false`.
+    /// Previously-set ``keepDownloaded`` on the directory must also survive.
+    ///
+    func testHandlePagedReadResultsTargetDirectoryRecordsVisitAndKeepsKeepDownloaded() {
+        let dbManager = Self.dbManager
+        debugPrint(dbManager.ncDatabase())
+
+        var seeded = remoteFolder.toItemMetadata(account: Self.account)
+        seeded.keepDownloaded = true
+        seeded.visitedDirectory = false
+        dbManager.addItemMetadata(seeded)
+
+        let parentNKFile = remoteFolder.toNKFile()
+        let (_, error) = Enumerator.handlePagedReadResults(
+            files: [parentNKFile], pageIndex: 0, dbManager: dbManager
+        )
+        XCTAssertNil(error)
+
+        let stored = dbManager.itemMetadata(ocId: parentNKFile.ocId)
+        XCTAssertEqual(
+            stored?.visitedDirectory, true,
+            "Page-0 target directory must be flagged as visited by the current request."
+        )
+        XCTAssertEqual(
+            stored?.keepDownloaded, true,
+            "Page-0 target directory must retain its existing keepDownloaded flag."
+        )
+    }
+
     func testWorkingSetEnumerateChanges() async throws {
         // This test verifies that `enumerateChanges` for the working set correctly
         // queries the local database for changes since the provided sync anchor date.

shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/KeepDownloadedRecursiveTests.swift

@@ -455,4 +455,90 @@ final class KeepDownloadedRecursiveTests: NextcloudFileProviderKitTestCase {
             "A sibling outside the pinned folder must not inherit keepDownloaded."
         )
     }
+
+    ///
+    /// End-to-end regression for #9923: a paginated read that lands on
+    /// already-pinned descendants must not silently clear their flag. Before
+    /// the fix, the bulk write at the end of
+    /// ``Enumerator.handlePagedReadResults`` used ``addItemMetadata`` which
+    /// replaces rows wholesale via Realm's `update: .all`, dropping every
+    /// local-only field — including ``keepDownloaded``. The user's pin walk
+    /// runs without pagination, but the OS-driven `enumerateItems` that
+    /// follows uses pagination, so a recently-pinned subtree was reliably
+    /// undone.
+    ///
+    func testPaginatedReadDoesNotClobberRecursivelyPinnedDescendants() throws {
+        // Pin the seeded tree the way `Item.set(keepDownloaded:domain:)` does.
+        let folderMetadata = try seedTree()
+        _ = try Self.dbManager.set(keepDownloaded: true, for: folderMetadata)
+        for child in Self.dbManager.childItems(directoryMetadata: folderMetadata) {
+            _ = try Self.dbManager.set(keepDownloaded: true, for: child)
+        }
+
+        // Build NKFiles with ocIds matching the seeded rows so the
+        // preservation lookup hits. The `MockRemoteItem` parent chain
+        // provides the right `serverUrl` on each `NKFile`.
+        let parent = MockRemoteItem(
+            identifier: "files-parent",
+            name: "files",
+            remotePath: "https://cloud.example.com/files",
+            directory: true,
+            account: Self.account.ncKitAccount,
+            username: Self.account.username,
+            userId: Self.account.id,
+            serverUrl: Self.account.serverUrl
+        )
+        let folderMock = MockRemoteItem(
+            identifier: "folder-1",
+            name: "documents",
+            remotePath: "https://cloud.example.com/files/documents",
+            directory: true,
+            account: Self.account.ncKitAccount,
+            username: Self.account.username,
+            userId: Self.account.id,
+            serverUrl: Self.account.serverUrl
+        )
+        folderMock.parent = parent
+        let directChildMock = MockRemoteItem(
+            identifier: "direct-child-file",
+            name: "report.pdf",
+            remotePath: "https://cloud.example.com/files/documents/report.pdf",
+            account: Self.account.ncKitAccount,
+            username: Self.account.username,
+            userId: Self.account.id,
+            serverUrl: Self.account.serverUrl
+        )
+        directChildMock.parent = folderMock
+        let subfolderMock = MockRemoteItem(
+            identifier: "subfolder-1",
+            name: "nested",
+            remotePath: "https://cloud.example.com/files/documents/nested",
+            directory: true,
+            account: Self.account.ncKitAccount,
+            username: Self.account.username,
+            userId: Self.account.id,
+            serverUrl: Self.account.serverUrl
+        )
+        subfolderMock.parent = folderMock
+
+        // Simulate the page-0 paginated PROPFIND response Finder would issue
+        // for the pinned folder.
+        let (_, error) = Enumerator.handlePagedReadResults(
+            files: [folderMock.toNKFile(), directChildMock.toNKFile(), subfolderMock.toNKFile()],
+            pageIndex: 0,
+            dbManager: Self.dbManager
+        )
+        XCTAssertNil(error)
+
+        // Both directly re-enumerated descendants and the deeper file
+        // (untouched by this PROPFIND but pinned in step 1) must remain
+        // pinned after the paginated overwrite.
+        for ocId in ["folder-1", "direct-child-file", "subfolder-1", "deep-file"] {
+            let stored = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: ocId))
+            XCTAssertTrue(
+                stored.keepDownloaded,
+                "\(ocId) must remain pinned after a paginated read overwrites the row (#9923)."
+            )
+        }
+    }
 }