Skip to content

Commit 9ac8d75

Browse files
authored
Merge pull request #9981 from nextcloud/i2h3/fix/9891-evict-pinned
Fix File Provider Item Eviction
2 parents 271efdd + cc0d9a4 commit 9ac8d75

8 files changed

Lines changed: 598 additions & 80 deletions

File tree

shell_integration/MacOSX/NextcloudFileProviderKit/Package.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ import PackageDescription
66
let package = Package(
77
name: "NextcloudFileProviderKit",
88
platforms: [
9-
.iOS(.v16),
10-
.macOS(.v13),
11-
.visionOS(.v1)
9+
.macOS(.v13)
1210
],
1311
products: [
1412
// Products define the executables and libraries a package produces, making them visible to other packages.

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@ public extension FilesDatabaseManager {
2121
.toUnmanagedResults()
2222
}
2323

24+
///
25+
/// Immediate children of a directory — i.e. items whose parent ``serverUrl``
26+
/// equals the directory's full path. Unlike ``childItems(directoryMetadata:)``
27+
/// this does not recurse into descendants.
28+
///
29+
func immediateChildItems(directoryMetadata: SendableItemMetadata) -> [SendableItemMetadata] {
30+
let directoryServerUrl = fullServerPathUrl(for: directoryMetadata)
31+
32+
return itemMetadatas
33+
.where { $0.serverUrl == directoryServerUrl }
34+
.toUnmanagedResults()
35+
}
36+
2437
func childItemCount(directoryMetadata: SendableItemMetadata) -> Int {
2538
let directoryServerUrl = fullServerPathUrl(for: directoryMetadata)
2639
return itemMetadatas

shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Extension/FileProviderExtension+CustomActions.swift

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,11 @@ extension FileProviderExtension: NSFileProviderCustomAction {
4646

4747
return Progress()
4848
case "com.nextcloud.desktopclient.FileProviderExt.KeepDownloadedAction":
49-
return performKeepDownloadedAction(
50-
keepDownloaded: true,
51-
onItemsWithIdentifiers: itemIdentifiers,
52-
completionHandler: completionHandler
53-
)
49+
return performKeepDownloadedAction(keepDownloaded: true, onItemsWithIdentifiers: itemIdentifiers, completionHandler: completionHandler)
5450
case "com.nextcloud.desktopclient.FileProviderExt.AutoEvictAction":
55-
return performKeepDownloadedAction(
56-
keepDownloaded: false,
57-
onItemsWithIdentifiers: itemIdentifiers,
58-
completionHandler: completionHandler
59-
)
51+
return performKeepDownloadedAction(keepDownloaded: false, onItemsWithIdentifiers: itemIdentifiers, completionHandler: completionHandler)
52+
case "com.nextcloud.desktopclient.FileProviderExt.EvictAction":
53+
return performEvictAction(onItemsWithIdentifiers: itemIdentifiers, completionHandler: completionHandler)
6054
default:
6155
logger.error("Unsupported action: \(actionIdentifier.rawValue)")
6256
completionHandler(NSError(domain: NSCocoaErrorDomain, code: NSFeatureUnsupportedError))
@@ -129,4 +123,77 @@ extension FileProviderExtension: NSFileProviderCustomAction {
129123
}
130124
return progress
131125
}
126+
127+
///
128+
/// Force the materialized payload of an item to be removed (made dataless), even when the item is pinned via "Always keep downloaded" (#9891).
129+
///
130+
/// macOS refuses `evictItem` on items whose `contentPolicy` is`.downloadEagerlyAndKeepDownloaded`.
131+
/// To honour the user's explicit "Remove download" gesture, we first clear the keep-downloaded flag — which flips `contentPolicy` back to `.inherited` and signals the framework — and then evict.
132+
/// The unpin is propagated to descendants by `Item.set(keepDownloaded:domain:)` (matches `AutoEvictAction` semantics), so directories behave consistently with the pin counterpart.
133+
///
134+
private func performEvictAction(onItemsWithIdentifiers itemIdentifiers: [NSFileProviderItemIdentifier], completionHandler: @Sendable @escaping ((any Error)?) -> Void) -> Progress {
135+
guard let ncAccount else {
136+
logger.error("Not removing downloads because account is not set up yet.")
137+
completionHandler(NSFileProviderError(.notAuthenticated))
138+
return Progress()
139+
}
140+
141+
guard let dbManager else {
142+
logger.error("Not removing downloads because database is unreachable.")
143+
completionHandler(NSFileProviderError(.cannotSynchronize))
144+
return Progress()
145+
}
146+
147+
guard let manager else {
148+
logger.error("Not removing downloads because file provider manager is not available.")
149+
completionHandler(NSFileProviderError(.providerNotFound))
150+
return Progress()
151+
}
152+
153+
let progress = Progress()
154+
155+
if itemIdentifiers.isEmpty {
156+
logger.info("No items to process for remove download action.")
157+
completionHandler(nil)
158+
return progress
159+
}
160+
161+
progress.totalUnitCount = Int64(itemIdentifiers.count)
162+
163+
Task {
164+
let localNcKit = self.ncKit
165+
let localDomain = self.domain
166+
167+
do {
168+
try await withThrowingTaskGroup(of: Void.self) { group in
169+
for identifier in itemIdentifiers {
170+
group.addTask {
171+
guard let item = await Item.storedItem(identifier: identifier, account: ncAccount, remoteInterface: localNcKit, dbManager: dbManager, log: self.log) else {
172+
throw NSError.fileProviderErrorForNonExistentItem(withIdentifier: identifier)
173+
}
174+
175+
// Clear keep-downloaded so that contentPolicy changes from `.downloadEagerlyAndKeepDownloaded` back to `.inherited`. `set(keepDownloaded:)` awaits the framework's acknowledgement of the modification, so by the time it returns the policy refresh has propagated.
176+
if item.keepDownloaded {
177+
try await item.set(keepDownloaded: false, domain: localDomain)
178+
}
179+
180+
try await manager.evictItem(identifier: identifier)
181+
}
182+
}
183+
184+
for try await _ in group {
185+
progress.completedUnitCount += 1
186+
}
187+
}
188+
189+
logger.info("All items successfully processed by evict action.")
190+
completionHandler(nil)
191+
} catch {
192+
logger.error("Error during eviction: \(error.localizedDescription)")
193+
completionHandler(error)
194+
}
195+
}
196+
197+
return progress
198+
}
132199
}

shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+KeepDownloaded.swift

Lines changed: 138 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,11 @@ public extension Item {
2828
_ = try dbManager.set(keepDownloaded: keepDownloaded, for: metadata)
2929

3030
guard let manager = NSFileProviderManager(for: domain) else {
31-
if #available(iOS 17.1, macOS 14.1, *) {
31+
if #available(macOS 14.1, *) {
3232
throw NSFileProviderError(.providerDomainNotFound)
3333
} else {
3434
let providerDomainNotFoundErrorCode = -2013
35-
throw NSError(
36-
domain: NSFileProviderErrorDomain,
37-
code: providerDomainNotFoundErrorCode,
38-
userInfo: [NSLocalizedDescriptionKey: "Failed to get manager for domain."]
39-
)
35+
throw NSError(domain: NSFileProviderErrorDomain, code: providerDomainNotFoundErrorCode, userInfo: [NSLocalizedDescriptionKey: "Failed to get manager for domain."])
4036
}
4137
}
4238

@@ -53,13 +49,17 @@ public extension Item {
5349
try await enumerateSubtreeBreadthFirst(domain: domain)
5450
}
5551

56-
try await signalKeepDownloaded(
57-
keepDownloaded: keepDownloaded,
58-
identifier: itemIdentifier,
59-
isDirectory: metadata.directory,
60-
isDownloaded: isDownloaded,
61-
manager: manager
62-
)
52+
try await signalKeepDownloaded(keepDownloaded: keepDownloaded, identifier: itemIdentifier, isDirectory: metadata.directory, isDownloaded: isDownloaded, manager: manager)
53+
54+
// When disabling, fragment the chain of pinned ancestors above this
55+
// item so the unpin actually takes effect against
56+
// `.downloadEagerlyAndKeepDownloaded` inheritance (#9891). Siblings at
57+
// every level retain their pin via the flag already set on them by
58+
// the original recursive enable; we only need to flip the path
59+
// ancestors themselves to `.inherited`.
60+
if !keepDownloaded {
61+
try await fragmentPathToRoot(manager: manager)
62+
}
6363

6464
guard metadata.directory else { return }
6565

@@ -103,20 +103,132 @@ public extension Item {
103103
/// already-downloaded files, and the disable path — just bump
104104
/// `lastUsedDate` so the framework re-evaluates eviction/pin state.
105105
///
106-
private func signalKeepDownloaded(
107-
keepDownloaded: Bool,
108-
identifier: NSFileProviderItemIdentifier,
109-
isDirectory: Bool,
110-
isDownloaded: Bool,
111-
manager: NSFileProviderManager
112-
) async throws {
106+
private func signalKeepDownloaded(keepDownloaded: Bool, identifier: NSFileProviderItemIdentifier, isDirectory: Bool, isDownloaded: Bool, manager: NSFileProviderManager) async throws {
113107
if keepDownloaded, !isDirectory, !isDownloaded {
114108
try await manager.requestDownloadForItem(withIdentifier: identifier)
115109
} else {
116-
try await manager.requestModification(
117-
of: [.lastUsedDate], forItemWithIdentifier: identifier
118-
)
110+
try await manager.requestModification(of: [.lastUsedDate], forItemWithIdentifier: identifier)
111+
}
112+
}
113+
114+
///
115+
/// Walk the parent chain of this item, flipping every pinned ancestor's
116+
/// keep-downloaded flag to `false` so its `contentPolicy` reverts from
117+
/// `.downloadEagerlyAndKeepDownloaded` to `.inherited`.
118+
///
119+
/// Why: the framework refuses `evictItem` on items whose effective
120+
/// content policy is `.downloadEagerlyAndKeepDownloaded`, and that policy
121+
/// inherits down the tree. Recursive "Always keep downloaded" sets the
122+
/// strict policy on every ancestor; unpinning a deep descendant
123+
/// in isolation leaves the descendant's `keepDownloaded == false` but its
124+
/// effective policy unchanged because some ancestor still resolves to the
125+
/// strict value. To actually free the descendant we have to cut the
126+
/// ancestors out of the strict-pin chain. Siblings at each level retain
127+
/// their pin via the flag already set on them by the original recursive
128+
/// enable, so no sibling write is needed (#9891).
129+
///
130+
/// The walk stops at the first ancestor that is not pinned; if no pinned
131+
/// ancestor exists, this is a no-op (the unpin gesture targets the pin
132+
/// root itself or there was no pin tree).
133+
///
134+
private func fragmentPathToRoot(manager: NSFileProviderManager) async throws {
135+
let outcome = fragmentPathToRootInDatabase()
136+
137+
// Cousins newly transitioned from `.inherited` (under a strict-pinned
138+
// ancestor) to an explicit pin must be told to the framework so it
139+
// refreshes their `contentPolicy` to `.downloadEagerlyAndKeepDownloaded`
140+
// — otherwise the unpin of the path ancestors would silently take
141+
// them down with the path.
142+
for cousin in outcome.newlyPinnedCousins {
143+
do {
144+
try await signalKeepDownloaded(keepDownloaded: true, identifier: NSFileProviderItemIdentifier(cousin.ocId), isDirectory: cousin.directory, isDownloaded: cousin.downloaded, manager: manager)
145+
} catch {
146+
logger.error("Could not signal newly-pinned cousin during unpin-path fragmentation.", [.item: cousin.ocId, .name: cousin.fileName, .error: error])
147+
}
148+
}
149+
150+
// Apply ancestor flips top-down (pin root first). Order does not affect
151+
// correctness — each ancestor is independent — but matches the
152+
// conceptual order of "cut from the top of the strict-pin chain
153+
// downward".
154+
for ancestor in outcome.unpinnedAncestors.reversed() {
155+
do {
156+
try await signalKeepDownloaded(keepDownloaded: false, identifier: NSFileProviderItemIdentifier(ancestor.ocId), isDirectory: ancestor.directory, isDownloaded: ancestor.downloaded, manager: manager)
157+
} catch {
158+
logger.error("Could not signal fragmented ancestor unpin to framework.", [.item: ancestor.ocId, .name: ancestor.fileName, .error: error])
159+
}
160+
}
161+
}
162+
163+
///
164+
/// Outcome of a database-only fragmentation walk.
165+
///
166+
/// `unpinnedAncestors` are listed bottom-up (immediate parent first, pin
167+
/// root last) — the order in which the walk discovered them. Callers that
168+
/// signal the framework should emit the unpin events top-down to mirror
169+
/// the conceptual "cut from the top of the strict-pin chain downward".
170+
///
171+
internal struct FragmentationOutcome {
172+
let unpinnedAncestors: [SendableItemMetadata]
173+
let newlyPinnedCousins: [SendableItemMetadata]
174+
}
175+
176+
///
177+
/// Database-only counterpart to ``fragmentPathToRoot(manager:)``.
178+
/// Walks parents, pins every off-path immediate child ("cousin") that
179+
/// isn't already pinned, then flips every pinned ancestor's flag to
180+
/// `false`. The framework-side signaling is the caller's job.
181+
///
182+
/// Why pin cousins explicitly instead of trusting the original recursive
183+
/// enable: the recursive enable runs once at pin time and only sees
184+
/// items the database knew about then. Items discovered later — e.g. a
185+
/// new server-side sibling surfaced by enumeration — enter with
186+
/// `keepDownloaded == false`. Without this explicit re-pin those cousins
187+
/// would silently lose their pin when the path ancestors flip to
188+
/// `.inherited` (#9891).
189+
///
190+
/// Exposed at module scope so tests can verify the flag mutations
191+
/// without needing a registered file provider domain.
192+
///
193+
internal func fragmentPathToRootInDatabase() -> FragmentationOutcome {
194+
// (ancestor, ocId of the child along the path) bottom-up.
195+
var pinnedAncestorsAndPathChildren: [(ancestor: SendableItemMetadata, pathChildOcId: String)] = []
196+
var cursor = metadata
197+
198+
while let parent = dbManager.parentDirectoryMetadataForItem(cursor) {
199+
guard parent.keepDownloaded else {
200+
break
201+
}
202+
203+
pinnedAncestorsAndPathChildren.append((ancestor: parent, pathChildOcId: cursor.ocId))
204+
cursor = parent
119205
}
206+
207+
var newlyPinnedCousins: [SendableItemMetadata] = []
208+
209+
for (ancestor, pathChildOcId) in pinnedAncestorsAndPathChildren {
210+
// Pin every immediate child of this ancestor except the one along
211+
// the path. Cousins already flagged are skipped to avoid noisy
212+
// DB writes (and noisy framework signals downstream).
213+
for cousin in dbManager.immediateChildItems(directoryMetadata: ancestor) where cousin.ocId != pathChildOcId && !cousin.keepDownloaded {
214+
do {
215+
_ = try dbManager.set(keepDownloaded: true, for: cousin)
216+
newlyPinnedCousins.append(cousin)
217+
} catch {
218+
logger.error("Could not pin off-path sibling while fragmenting unpin path.", [.item: cousin.ocId, .name: cousin.fileName, .error: error])
219+
}
220+
}
221+
222+
// Then flip the ancestor itself.
223+
do {
224+
_ = try dbManager.set(keepDownloaded: false, for: ancestor)
225+
} catch {
226+
logger.error("Could not clear keep-downloaded on ancestor while fragmenting unpin path.", [.item: ancestor.ocId, .name: ancestor.fileName, .error: error])
227+
continue
228+
}
229+
}
230+
231+
return FragmentationOutcome(unpinnedAncestors: pinnedAncestorsAndPathChildren.map(\.ancestor), newlyPinnedCousins: newlyPinnedCousins)
120232
}
121233

122234
///
@@ -153,20 +265,12 @@ public extension Item {
153265

154266
if let readError, readError != .success {
155267
if isTopLevel {
156-
logger.error(
157-
"Could not enumerate directory for keep-downloaded.",
158-
[.name: metadata.fileName, .url: remoteDirectoryPath, .error: readError]
159-
)
160-
throw readError.fileProviderError(
161-
handlingNoSuchItemErrorUsingItemIdentifier: itemIdentifier
162-
) ?? NSFileProviderError(.cannotSynchronize)
268+
logger.error("Could not enumerate directory for keep-downloaded.", [.name: metadata.fileName, .url: remoteDirectoryPath, .error: readError])
269+
throw readError.fileProviderError(handlingNoSuchItemErrorUsingItemIdentifier: itemIdentifier) ?? NSFileProviderError(.cannotSynchronize)
163270
} else {
164271
// A single failing descendant must not abort the whole
165272
// subtree — log and skip the rest of this branch.
166-
logger.error(
167-
"Could not enumerate descendant directory for keep-downloaded; skipping branch.",
168-
[.url: remoteDirectoryPath, .error: readError]
169-
)
273+
logger.error("Could not enumerate descendant directory for keep-downloaded; skipping branch.", [.url: remoteDirectoryPath, .error: readError])
170274
isTopLevel = false
171275
continue
172276
}

shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item.swift

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,23 @@ public final class Item: NSObject, NSFileProviderItem, Sendable {
4141
if permissions.contains("D") { // Deletable
4242
capabilities.insert(.allowsDeleting)
4343
}
44+
4445
if remoteSupportsTrash, !isLockFileName(filename) {
4546
capabilities.insert(.allowsTrashing)
4647
}
48+
4749
if permissions.contains("W"), !metadata.directory { // Updateable (file)
4850
capabilities.insert(.allowsWriting)
4951
}
52+
5053
if permissions.contains("NV") { // Updateable, renameable, moveable
5154
capabilities.formUnion([.allowsRenaming, .allowsReparenting])
5255

5356
if metadata.directory {
5457
capabilities.insert(.allowsAddingSubItems)
5558
}
5659
}
60+
5761
if permissions.contains("CK"), metadata.directory { // Folder not changeable but adding sub-files & -folders
5862
capabilities.insert(.allowsWriting)
5963
}
@@ -199,6 +203,12 @@ public final class Item: NSObject, NSFileProviderItem, Sendable {
199203

200204
userInfoDict["displayKeepDownloaded"] = !metadata.keepDownloaded
201205
userInfoDict["displayAllowAutoEvicting"] = metadata.keepDownloaded
206+
// Restricted to non-pinned items so the action only appears once the
207+
// framework has refreshed `contentPolicy` to `.inherited`. Both fields
208+
// are read from the same `Item` returned by `item(for:)`, so they
209+
// always agree — preventing the -2008 NonEvictable race that would
210+
// otherwise occur if we tried to evict while `requestModification`'s
211+
// unpin signal was still queued (#9891).
202212
userInfoDict["displayEvict"] = metadata.downloaded && !metadata.keepDownloaded
203213

204214
// https://docs.nextcloud.com/server/latest/developer_manual/client_apis/WebDAV/basic.html
@@ -209,13 +219,10 @@ public final class Item: NSObject, NSFileProviderItem, Sendable {
209219
return userInfoDict
210220
}
211221

212-
@available(macOS 13.0, iOS 16.0, visionOS 1.0, *)
213222
public var contentPolicy: NSFileProviderContentPolicy {
214-
#if os(macOS)
215-
if metadata.keepDownloaded {
216-
return .downloadEagerlyAndKeepDownloaded // Unavailable in iOS.
217-
}
218-
#endif
223+
if metadata.keepDownloaded {
224+
return .downloadEagerlyAndKeepDownloaded
225+
}
219226

220227
return .inherited
221228
}

0 commit comments

Comments
 (0)