Skip to content

Commit 53a3aad

Browse files
i2h3i2h3
committed
fix: Trash enumerates only locally trashed items.
- Introduced new schema property wasTrashedLocally on SendableItemMetadata. - Added corresponding schema version and migration. - Extended related data models accordingly. - Updated Enumerator to no longer enumerate remote trash items based on wasTrashedLocally. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
1 parent 2533a33 commit 53a3aad

19 files changed

Lines changed: 127 additions & 305 deletions

‎Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public final class FilesDatabaseManager: Sendable {
4040
)
4141
}
4242

43-
private static let schemaVersion = SchemaVersion.addedIsLockFileOfLocalOriginToRealmItemMetadata
43+
private static let schemaVersion = SchemaVersion.addedWasTrashedLocallyToRealmItemMetadata
4444
let logger: FileProviderLogger
4545
let account: Account
4646

@@ -134,6 +134,15 @@ public final class FilesDatabaseManager: Sendable {
134134
}
135135
}
136136

137+
if oldSchemaVersion == SchemaVersion.addedIsLockFileOfLocalOriginToRealmItemMetadata.rawValue {
138+
migration.enumerateObjects(ofType: RealmItemMetadata.className()) { _, newObject in
139+
guard let newObject else {
140+
return
141+
}
142+
143+
newObject["wasTrashedLocally"] = false
144+
}
145+
}
137146
},
138147
objectTypes: [RealmItemMetadata.self, RemoteFileChunk.self]
139148
)
@@ -520,7 +529,7 @@ public final class FilesDatabaseManager: Sendable {
520529
do {
521530
try database.write {
522531
database.add(RealmItemMetadata(value: metadata), update: .all)
523-
logger.debug("Added item metadata.", [.item: metadata.ocId, .name: metadata.name, .url: metadata.serverUrl])
532+
logger.debug("Added item metadata.", [.item: metadata.ocId, .name: metadata.fileName, .url: metadata.serverUrl])
524533
}
525534
} catch {
526535
logger.error("Failed to add item metadata.", [.item: metadata.ocId, .name: metadata.name, .url: metadata.serverUrl, .error: error])

‎Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ enum SchemaVersion: UInt64 {
99
case deletedLocalFileMetadata = 200
1010
case addedLockTokenPropertyToRealmItemMetadata = 201
1111
case addedIsLockFileOfLocalOriginToRealmItemMetadata = 202
12+
case addedWasTrashedLocallyToRealmItemMetadata = 203
1213
}

‎Sources/NextcloudFileProviderKit/Enumeration/Enumerator+Trash.swift‎

Lines changed: 20 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -5,96 +5,29 @@
55
import NextcloudKit
66

77
extension Enumerator {
8-
static func completeEnumerationObserver(
9-
_ observer: NSFileProviderEnumerationObserver,
10-
account: Account,
11-
remoteInterface: RemoteInterface,
12-
dbManager: FilesDatabaseManager,
13-
numPage: Int,
14-
trashItems: [NKTrash],
15-
log: any FileProviderLogging
16-
) {
17-
var metadatas = [SendableItemMetadata]()
18-
for trashItem in trashItems {
19-
let metadata = trashItem.toItemMetadata(account: account)
20-
dbManager.addItemMetadata(metadata)
21-
metadatas.append(metadata)
22-
}
23-
24-
Task { [metadatas] in
25-
let logger = FileProviderLogger(category: "Enumerator", log: log)
26-
27-
do {
28-
let items = try await metadatas.toFileProviderItems(account: account, remoteInterface: remoteInterface, dbManager: dbManager, log: log)
29-
30-
Task { @MainActor in
31-
observer.didEnumerate(items)
32-
logger.info("Did enumerate \(items.count) trash items.")
33-
observer.finishEnumerating(upTo: fileProviderPageforNumPage(numPage))
34-
}
35-
} catch {
36-
logger.error("Finishing enumeration with error.")
37-
Task { @MainActor in observer.finishEnumeratingWithError(error) }
38-
}
39-
}
40-
}
41-
42-
static func completeChangesObserver(
43-
_ observer: NSFileProviderChangeObserver,
44-
anchor: NSFileProviderSyncAnchor,
45-
account: Account,
46-
remoteInterface: RemoteInterface,
47-
dbManager: FilesDatabaseManager,
48-
trashItems: [NKTrash],
49-
log: any FileProviderLogging
50-
) async {
8+
///
9+
/// Change enumeration completion.
10+
///
11+
/// `NKTrash` items do not have an ETag. We assume they cannot be modified while they are in the trash. So we will just check by their `ocId`.
12+
/// Newly added items by deletion on the server side or another client are not of interested and we do not want to display them in the local trash.
13+
/// In the end, only the remotely and permanently deleted items are of interest.
14+
///
15+
static func completeChangesObserver(_ observer: NSFileProviderChangeObserver, anchor: NSFileProviderSyncAnchor, account: Account, dbManager: FilesDatabaseManager, remoteTrashItems: [NKTrash], log: any FileProviderLogging) async {
5116
let logger = FileProviderLogger(category: "Enumerator", log: log)
52-
var newTrashedItems = [NSFileProviderItem]()
53-
54-
// NKTrash items do not have an etag ; we assume they cannot be modified while they are in
55-
// the trash, so we will just check by ocId
56-
var existingTrashedItems = dbManager.trashedItemMetadatas(account: account)
57-
58-
for trashItem in trashItems {
59-
if let existingTrashItemIndex = existingTrashedItems.firstIndex(
60-
where: { $0.ocId == trashItem.ocId }
61-
) {
62-
existingTrashedItems.remove(at: existingTrashItemIndex)
63-
continue
64-
}
65-
66-
let metadata = trashItem.toItemMetadata(account: account)
67-
dbManager.addItemMetadata(metadata)
68-
69-
let item = await Item(
70-
metadata: metadata,
71-
parentItemIdentifier: .trashContainer,
72-
account: account,
73-
remoteInterface: remoteInterface,
74-
dbManager: dbManager,
75-
remoteSupportsTrash: remoteInterface.supportsTrash(account: account),
76-
log: log
77-
)
78-
newTrashedItems.append(item)
79-
80-
logger.debug("Will enumerate changed trash item.", [.item: metadata.ocId, .name: metadata.fileName])
81-
}
82-
83-
let deletedTrashedItemsIdentifiers = existingTrashedItems.map {
84-
NSFileProviderItemIdentifier($0.ocId)
85-
}
86-
if !deletedTrashedItemsIdentifiers.isEmpty {
87-
for itemIdentifier in deletedTrashedItemsIdentifiers {
88-
dbManager.deleteItemMetadata(ocId: itemIdentifier.rawValue)
89-
}
90-
91-
logger.debug("Will enumerate deleted trashed items: \(deletedTrashedItemsIdentifiers)")
92-
observer.didDeleteItems(withIdentifiers: deletedTrashedItemsIdentifiers)
17+
let localIdentifiers = dbManager.trashedItemMetadatas(account: account).map(\.ocId)
18+
let localSet = Set(localIdentifiers)
19+
let remoteIdentifiers = remoteTrashItems.map(\.ocId)
20+
let remoteSet = Set(remoteIdentifiers)
21+
let orphanedSet = localSet.subtracting(remoteSet)
22+
let orphanedIdentifiers = orphanedSet.map { NSFileProviderItemIdentifier($0) }
23+
24+
for identifier in orphanedSet {
25+
logger.info("Permanently deleting remote trash item which could not be matched with a local one.", [.item: identifier])
26+
dbManager.deleteItemMetadata(ocId: identifier)
9327
}
9428

95-
if !newTrashedItems.isEmpty {
96-
observer.didUpdate(newTrashedItems)
97-
}
29+
observer.didDeleteItems(withIdentifiers: orphanedIdentifiers)
9830
observer.finishEnumeratingChanges(upTo: anchor, moreComing: false)
31+
logger.debug("Finished enumerating remote changes in trash.")
9932
}
10033
}

‎Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift‎

Lines changed: 6 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -76,21 +76,8 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable {
7676
public func enumerateItems(for observer: NSFileProviderEnumerationObserver, startingAt page: NSFileProviderPage) {
7777
logger.info("Received enumerate items request for enumerator with user", [.account: account.ncKitAccount, .url: serverUrl])
7878

79-
/*
80-
- inspect the page to determine whether this is an initial or a follow-up request (TODO)
81-
82-
If this is an enumerator for a directory, the root container or all directories:
83-
- perform a server request to fetch directory contents
84-
If this is an enumerator for the working set:
85-
- perform a server request to update your local database
86-
- fetch the working set from your local database
87-
88-
- inform the observer about the items returned by the server (possibly multiple times)
89-
- inform the observer that you are finished with this page
90-
*/
91-
9279
if enumeratedItemIdentifier == .trashContainer {
93-
logger.info("Enumerating trash.", [.account: account.ncKitAccount, .url: serverUrl])
80+
logger.info("Enumerating items in trash.", [.account: account.ncKitAccount, .url: serverUrl])
9481

9582
Task { [weak self] in
9683
guard let self else {
@@ -111,42 +98,11 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable {
11198
return
11299
}
113100

114-
let domain = domain
115-
let enumeratedItemIdentifier = enumeratedItemIdentifier
116-
117-
let (_, trashedItems, _, trashReadError) = await remoteInterface.listingTrashAsync(
118-
filename: nil,
119-
showHiddenFiles: true,
120-
account: account.ncKitAccount,
121-
options: .init(),
122-
taskHandler: { task in
123-
if let domain {
124-
NSFileProviderManager(for: domain)?.register(
125-
task,
126-
forItemWithIdentifier: enumeratedItemIdentifier,
127-
completionHandler: { _ in }
128-
)
129-
}
130-
}
131-
)
132-
133-
guard trashReadError == .success else {
134-
let error = trashReadError.fileProviderError(handlingNoSuchItemErrorUsingItemIdentifier: enumeratedItemIdentifier) ?? NSFileProviderError(.cannotSynchronize)
135-
observer.finishEnumeratingWithError(error)
136-
return
137-
}
138-
139-
Self.completeEnumerationObserver(
140-
observer,
141-
account: account,
142-
remoteInterface: remoteInterface,
143-
dbManager: dbManager,
144-
numPage: 1,
145-
trashItems: trashedItems ?? [],
146-
log: logger.log
147-
)
101+
// We only want to list items deleted on the local device.
102+
// That cannot happen before the initial content enumeration for a file provider domain because the latter does not exist yet.
103+
// Hence the initial trash content enumeration can be finished with an empty set.
104+
observer.finishEnumerating(upTo: Self.fileProviderPageforNumPage(1))
148105
}
149-
150106
return
151107
}
152108

@@ -334,9 +290,8 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable {
334290
observer,
335291
anchor: anchor,
336292
account: account,
337-
remoteInterface: remoteInterface,
338293
dbManager: dbManager,
339-
trashItems: trashedItems ?? [],
294+
remoteTrashItems: trashedItems ?? [],
340295
log: logger.log
341296
)
342297
}

‎Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ extension NKFile {
1515
return fileUrl == urlString
1616
}
1717

18-
func toItemMetadata(uploaded: Bool = true) -> SendableItemMetadata {
18+
func toItemMetadata(uploaded: Bool = true, wasTrashedLocally: Bool = false) -> SendableItemMetadata {
1919
let creationDate = creationDate ?? date
2020
let uploadDate = uploadDate ?? date
2121

@@ -88,7 +88,8 @@ extension NKFile {
8888
uploadDate: uploadDate as Date,
8989
urlBase: urlBase,
9090
user: user,
91-
userId: userId
91+
userId: userId,
92+
wasTrashedLocally: wasTrashedLocally
9293
)
9394
}
9495
}

‎Sources/NextcloudFileProviderKit/Extensions/NKTrash+Extensions.swift‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import Foundation
55
import NextcloudKit
66

77
extension NKTrash {
8-
func toItemMetadata(account: Account) -> SendableItemMetadata {
8+
///
9+
/// Convert a trashed item representation into sendable item metadata.
10+
///
11+
func toItemMetadata(account: Account, wasTrashedLocally: Bool = false) -> SendableItemMetadata {
912
SendableItemMetadata(
1013
ocId: ocId,
1114
account: account.ncKitAccount,
@@ -35,7 +38,8 @@ extension NKTrash {
3538
trashbinDeletionTime: trashbinDeletionTime,
3639
urlBase: account.serverUrl,
3740
user: account.username,
38-
userId: account.id
41+
userId: account.id,
42+
wasTrashedLocally: wasTrashedLocally
3943
)
4044
}
4145
}

‎Sources/NextcloudFileProviderKit/Item/Item+Create.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,8 @@ public extension Item {
213213
uploaded: true,
214214
urlBase: account.serverUrl,
215215
user: account.username,
216-
userId: account.id
216+
userId: account.id,
217+
wasTrashedLocally: false
217218
)
218219

219220
dbManager.addItemMetadata(newMetadata)

‎Sources/NextcloudFileProviderKit/Item/Item+Ignored.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ extension Item {
5252
uploaded: false,
5353
urlBase: account.serverUrl,
5454
user: account.username,
55-
userId: account.id
55+
userId: account.id,
56+
wasTrashedLocally: false
5657
)
5758

5859
dbManager.addItemMetadata(metadata)

‎Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ extension Item {
126126
uploaded: false,
127127
urlBase: account.serverUrl,
128128
user: account.username,
129-
userId: account.id
129+
userId: account.id,
130+
wasTrashedLocally: false
130131
)
131132

132133
dbManager.addItemMetadata(metadata)

‎Sources/NextcloudFileProviderKit/Item/Item+Modify.swift‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -656,13 +656,13 @@ public extension Item {
656656

657657
return (modifiedItem, nil)
658658
} else if changedFields.contains(.parentItemIdentifier) && newParentItemIdentifier == .trashContainer {
659-
let (_, capabilities, _, error) = await remoteInterface.currentCapabilities(
660-
account: account, options: .init(), taskHandler: { _ in }
661-
)
659+
let (_, capabilities, _, error) = await remoteInterface.currentCapabilities(account: account, options: .init(), taskHandler: { _ in })
660+
662661
guard let capabilities, error == .success else {
663662
logger.error("Could not acquire capabilities during item move to trash, won't proceed.", [.item: modifiedItem, .error: error])
664663
return (nil, error.fileProviderError)
665664
}
665+
666666
guard capabilities.files?.undelete == true else {
667667
logger.error("Cannot delete item as server does not support trashing.", [.item: modifiedItem])
668668
return (nil, NSError(domain: NSCocoaErrorDomain, code: NSFeatureUnsupportedError))
@@ -672,15 +672,8 @@ public extension Item {
672672
// Rename the item if necessary before doing the trashing procedures
673673
if changedFields.contains(.filename) {
674674
let currentParentItemRemotePath = modifiedItem.metadata.serverUrl
675-
let preTrashingRenamedRemotePath =
676-
currentParentItemRemotePath + "/" + itemTarget.filename
677-
let (renameModifiedItem, renameError) = await modifiedItem.move(
678-
newFileName: itemTarget.filename,
679-
newRemotePath: preTrashingRenamedRemotePath,
680-
newParentItemIdentifier: modifiedItem.parentItemIdentifier,
681-
newParentItemRemotePath: currentParentItemRemotePath,
682-
dbManager: dbManager
683-
)
675+
let preTrashingRenamedRemotePath = currentParentItemRemotePath + "/" + itemTarget.filename
676+
let (renameModifiedItem, renameError) = await modifiedItem.move(newFileName: itemTarget.filename, newRemotePath: preTrashingRenamedRemotePath, newParentItemIdentifier: modifiedItem.parentItemIdentifier, newParentItemRemotePath: currentParentItemRemotePath, dbManager: dbManager)
684677

685678
guard renameError == nil, let renameModifiedItem else {
686679
logger.error("Could not rename pre-trash item.", [.item: modifiedItem.itemIdentifier, .error: error])
@@ -690,10 +683,12 @@ public extension Item {
690683
modifiedItem = renameModifiedItem
691684
}
692685

693-
let (trashedItem, trashingError) = await Self.trash(
694-
modifiedItem, account: account, dbManager: dbManager, domain: domain, log: logger.log
695-
)
696-
guard trashingError == nil else { return (modifiedItem, trashingError) }
686+
let (trashedItem, trashingError) = await Self.trash(modifiedItem, account: account, dbManager: dbManager, domain: domain, log: logger.log)
687+
688+
guard trashingError == nil else {
689+
return (modifiedItem, trashingError)
690+
}
691+
697692
modifiedItem = trashedItem
698693
} else if changedFields.contains(.filename) || changedFields.contains(.parentItemIdentifier) {
699694
// Recover the item first

0 commit comments

Comments
 (0)