From 0043b2c79e0ca18bcc54c8ce268138f8a95f53f8 Mon Sep 17 00:00:00 2001 From: Rello Date: Wed, 26 Aug 2026 17:55:46 +0200 Subject: [PATCH] fix(fileprovider): preserve content version across file locks Signed-off-by: Rello Assisted-by: Codex:GPT-5 --- .../xcshareddata/swiftpm/Package.resolved | 6 +- .../NextcloudFileProviderKit/Package.swift | 2 +- .../Database/FilesDatabaseManager.swift | 18 +++- .../Database/SchemaVersion.swift | 1 + .../Enumeration/Enumerator+SyncEngine.swift | 3 + .../Item/Item+LockFile.swift | 14 ++++ .../Item/Item+Modify.swift | 11 +-- .../NextcloudFileProviderKit/Item/Item.swift | 32 ++++++-- .../Metadata/ItemMetadata.swift | 1 + .../Metadata/RealmItemMetadata.swift | 2 + .../Metadata/SendableItemMetadata.swift | 4 + .../Tests/Interface/MockRemoteInterface.swift | 12 ++- .../FilesDatabaseManagerTests.swift | 20 +++++ .../ItemCreateTests.swift | 65 +++++++++++++++ .../ItemModifyTests.swift | 82 +++++++++++++++---- .../ItemPropertyTests.swift | 53 ++++++++++++ 16 files changed, 291 insertions(+), 35 deletions(-) diff --git a/Nextcloud Desktop Client.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Nextcloud Desktop Client.xcworkspace/xcshareddata/swiftpm/Package.resolved index 5e9abb7522213..cb36c4a017c58 100644 --- a/Nextcloud Desktop Client.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Nextcloud Desktop Client.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "09a77c502e437790be000de1eb2b894b4b1d352b35dd71fb31e6522346845201", + "originHash" : "13284141ea8e576b02ea91c95bdb7c5e812eedbff3d2153b4d75493298360fe0", "pins" : [ { "identity" : "alamofire", @@ -33,8 +33,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/nextcloud/NextcloudKit", "state" : { - "revision" : "71fe462e9bc4620325f4f12656f54849e2316e95", - "version" : "7.4.0" + "revision" : "2c86f8b3af59b51f60bfcc8620f59f845a5554fb", + "version" : "7.5.0" } }, { diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Package.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Package.swift index 8afae0dbee687..557a10f098f38 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Package.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Package.swift @@ -21,7 +21,7 @@ let package = Package( ], dependencies: [ .package(url: "https://github.com/nextcloud/NextcloudCapabilitiesKit.git", from: "2.5.0"), - .package(url: "https://github.com/nextcloud/NextcloudKit", from: "7.3.5"), + .package(url: "https://github.com/nextcloud/NextcloudKit", from: "7.5.0"), .package(url: "https://github.com/nicklockwood/SwiftFormat", from: "0.55.0"), .package(url: "https://github.com/realm/realm-swift.git", from: "20.0.4") ], diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index 73ac5e82b6759..f462fcafb27fd 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -31,7 +31,7 @@ public final class FilesDatabaseManager: Sendable { ) } - private static let schemaVersion = SchemaVersion.addedChangeDeliverySessions + private static let schemaVersion = SchemaVersion.addedFileProviderContentVersion let logger: FileProviderLogger let account: Account @@ -316,6 +316,10 @@ public final class FilesDatabaseManager: Sendable { for var updatedMetadata in updatedMetadatas { if let existingMetadata = existingByOcId[updatedMetadata.ocId] { + if updatedMetadata.etag == existingMetadata.etag { + updatedMetadata.fileProviderContentVersion = existingMetadata.fileProviderContentVersion + } + if existingMetadata.status == Status.normal.rawValue, !existingMetadata.isInSameDatabaseStoreableRemoteState(updatedMetadata) { let pathChanged = !updatedMetadata.hasSameLocation(as: existingMetadata) @@ -444,6 +448,10 @@ public final class FilesDatabaseManager: Sendable { } if let existing = itemMetadata(ocId: readTargetMetadata.ocId) { + if readTargetMetadata.etag == existing.etag { + readTargetMetadata.fileProviderContentVersion = existing.fileProviderContentVersion + } + if existing.status == Status.normal.rawValue, !existing.isInSameDatabaseStoreableRemoteState(readTargetMetadata) { @@ -602,7 +610,7 @@ 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``. + /// ``visitedDirectory``, ``lockToken``, and the content version File Provider has already seen. /// /// Mirrors the preservation set applied by /// ``processItemMetadatasToUpdate`` for non-paginated reads. Use this from @@ -641,6 +649,9 @@ public final class FilesDatabaseManager: Sendable { } toWrite.lockToken = existing.lockToken + if toWrite.etag == existing.etag { + toWrite.fileProviderContentVersion = existing.fileProviderContentVersion + } } else { // The ocId lookup missed. Before falling back to defaults from the // server payload, look for a single non-deleted, non-local-lock row @@ -673,6 +684,9 @@ public final class FilesDatabaseManager: Sendable { } toWrite.lockToken = existing.lockToken + if toWrite.etag == existing.etag { + toWrite.fileProviderContentVersion = existing.fileProviderContentVersion + } } else { // No prior row at this ocId or logical address: this is a // genuinely new item. Inherit the parent's "Always keep diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift index b97a2ebb7c1e7..460e48a0ca3b1 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift @@ -14,4 +14,5 @@ enum SchemaVersion: UInt64 { case addedExcludedFromSyncItems = 205 case addedPendingChunkUploadCleanup = 206 case addedChangeDeliverySessions = 207 + case addedFileProviderContentVersion = 208 } diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift index 956ca3dfd9272..5a9a7745c1cef 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift @@ -282,6 +282,9 @@ extension Enumerator { let isNew = existing == nil let newItems: [SendableItemMetadata] = isNew ? [metadata] : [] metadata.lockToken = existing?.lockToken + if metadata.etag == existing?.etag { + metadata.fileProviderContentVersion = existing?.fileProviderContentVersion + } let updatedItems: [SendableItemMetadata] = isNew ? [] : [metadata] metadata.downloaded = existing?.downloaded == true diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift index 49c1fe1e12013..5d252e65093e8 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift @@ -143,7 +143,21 @@ extension Item { targetMetadata.lockOwnerType = lock.ownerType.rawValue targetMetadata.lockTime = lock.time targetMetadata.lockTimeOut = lock.timeOut + if let etag = lock.etag { + // LOCK changes server metadata, not file bytes. Keep the content version + // File Provider already knows while adopting the lock response's etag. + if targetMetadata.fileProviderContentVersion == nil { + targetMetadata.fileProviderContentVersion = targetMetadata.etag + } + targetMetadata.etag = etag + } targetMetadata.lockToken = lock.token + // Ensure token-dependent capabilities are published even if the etag is unchanged. + targetMetadata.syncTime = Date() + } + + if let domain { + FileProviderChangeNotificationInterface(domain: domain, log: log).notifyChange() } } else { logger.error("Failed to find target item for acquired lock.", [.lock: lock]) diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift index 7646a5593c9ab..444ac1c719043 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift @@ -128,7 +128,6 @@ public extension Item { } var headers = [String: String]() - if let token = metadata.lockToken { headers["If"] = "<\(remotePath)> ()" } @@ -165,10 +164,11 @@ public extension Item { shouldSendIfMatch = true } - // We can only guard the write if we know the version to match against. When we - // do, a subsequent 412 is a real content conflict (below); when we don't, the - // upload stays unconditional and 412 keeps its previous stale-lock meaning. - let sentIfMatch = shouldSendIfMatch && baseEtag != nil + // A lock token is already an exclusive write precondition. Its acquisition also changes + // the server etag, while File Provider's base version intentionally remains the version the + // document was opened from. Do not combine that pre-lock etag with the current lock token. + // Without a lock token, keep using the etag as the optimistic-concurrency guard. + let sentIfMatch = shouldSendIfMatch && baseEtag != nil && metadata.lockToken == nil if sentIfMatch, let baseEtag { // Our stored etag is normalized (unquoted); Sabre/DAV compares If-Match // against the quoted resource ETag, so re-add the quotes. @@ -322,6 +322,7 @@ public extension Item { // "changed by another application" right after they save it. newMetadata.date = newContentModificationDate ?? date ?? metadata.date newMetadata.etag = etag ?? metadata.etag + newMetadata.fileProviderContentVersion = newMetadata.etag newMetadata.ocId = ocId newMetadata.size = size ?? 0 newMetadata.session = "" diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item.swift index ea67802f59b6f..db14716fd6085 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item.swift @@ -23,6 +23,14 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { private let displayFileActions: Bool private let remoteSupportsTrash: Bool + private var lockStateAllowsModifications: Bool { + metadata.lock == false || ( + metadata.lockOwnerType == NKLockType.token.rawValue && + metadata.ownerId == metadata.lockOwner && + metadata.lockToken != nil + ) + } + public var itemIdentifier: NSFileProviderItemIdentifier { NSFileProviderItemIdentifier(metadata.ocId) } @@ -37,7 +45,7 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { capabilities.insert(.allowsReading) } - if metadata.lock == false || (metadata.lock == true && metadata.lockOwnerType == NKLockType.token.rawValue && metadata.ownerId == metadata.lockOwner && metadata.lockToken != nil) { + if lockStateAllowsModifications { if permissions.contains("D") { // Deletable capabilities.insert(.allowsDeleting) } @@ -87,14 +95,21 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { // `metadataVersion` we previously handed the framework, the framework would treat // its cached snapshot as still valid, and the new derivations would never reach the // system — leaving e.g. the `displayOpenInBrowser` / `displayCopyInternalLink` - // userInfo keys missing on items enumerated by older builds. `contentVersion` stays - // bare-etag because the file content itself didn't change across the upgrade and - // bumping it would force the framework to re-download every materialised file. + // userInfo keys missing on items enumerated by older builds. `contentVersion` normally + // follows the etag, but lock-only etag transitions preserve the prior value because the + // file bytes did not change. Existing rows without that separately stored value fall back + // to the etag. // // See nextcloud/desktop#10065. let extensionVersion = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String ?? "" var metadataVersionString = "\(metadata.etag)|\(extensionVersion)" + // Reacquiring an existing lock can restore the local token without changing the etag. + // Include the resulting capability state so File Provider re-reads the item as writable. + if metadata.lock { + metadataVersionString += "|\(lockStateAllowsModifications)" + } + // A directory's `displayEvictDescendants` ("Remove downloaded items") // depends on whether it holds an evictable descendant file — state that // lives outside the folder's own etag. Fold that boolean into @@ -111,7 +126,8 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { metadataVersionString += "|\(dbManager.hasEvictableDescendantFile(directoryMetadata: metadata))" } - return NSFileProviderItemVersion(contentVersion: metadata.etag.data(using: .utf8)!, metadataVersion: metadataVersionString.data(using: .utf8)!) + let contentVersion = metadata.fileProviderContentVersion ?? metadata.etag + return NSFileProviderItemVersion(contentVersion: Data(contentVersion.utf8), metadataVersion: Data(metadataVersionString.utf8)) } public var filename: String { @@ -218,7 +234,11 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { ] } - if metadata.lock, metadata.lockOwnerType != NKLockType.user.rawValue || metadata.lockOwner != account.username, metadata.lockTimeOut ?? Date() > Date() { + if metadata.lock, + !lockStateAllowsModifications, + metadata.lockOwnerType != NKLockType.user.rawValue || metadata.lockOwner != account.username, + metadata.lockTimeOut ?? Date() > Date() + { return [ .userReadable ] diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift index 8cbace8ca136f..7cc9f6d2eea1d 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift @@ -39,6 +39,7 @@ public protocol ItemMetadata: Equatable { var downloadURL: String { get set } var e2eEncrypted: Bool { get set } var etag: String { get set } + var fileProviderContentVersion: String? { get set } var favorite: Bool { get set } var fileId: String { get set } var fileName: String { get set } // What the file's real file name is diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift index f499fc9769a0c..bc05841a561d7 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift @@ -27,6 +27,7 @@ class RealmItemMetadata: Object, ItemMetadata { @Persisted var downloadURL = "" @Persisted var e2eEncrypted: Bool = false @Persisted var etag = "" + @Persisted var fileProviderContentVersion: String? @Persisted var favorite: Bool = false @Persisted var fileId = "" @Persisted var fileName = "" // What the file's real file name is @@ -140,6 +141,7 @@ class RealmItemMetadata: Object, ItemMetadata { downloadURL = value.downloadURL e2eEncrypted = value.e2eEncrypted etag = value.etag + fileProviderContentVersion = value.fileProviderContentVersion favorite = value.favorite fileId = value.fileId fileNameView = value.fileNameView diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift index 4806bf898fa1a..ee22baa4295c6 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift @@ -27,6 +27,7 @@ public struct SendableItemMetadata: ItemMetadata, Codable, Sendable { public var downloadURL: String public var e2eEncrypted: Bool public var etag: String + public var fileProviderContentVersion: String? public var favorite: Bool public var fileId: String public var fileName: String @@ -95,6 +96,7 @@ public struct SendableItemMetadata: ItemMetadata, Codable, Sendable { downloadURL: String = "", e2eEncrypted: Bool, etag: String, + fileProviderContentVersion: String? = nil, favorite: Bool = false, fileId: String, fileName: String, @@ -162,6 +164,7 @@ public struct SendableItemMetadata: ItemMetadata, Codable, Sendable { self.downloadURL = downloadURL self.e2eEncrypted = e2eEncrypted self.etag = etag + self.fileProviderContentVersion = fileProviderContentVersion self.favorite = favorite self.fileId = fileId self.fileName = fileName @@ -231,6 +234,7 @@ public struct SendableItemMetadata: ItemMetadata, Codable, Sendable { downloadURL = value.downloadURL e2eEncrypted = value.e2eEncrypted etag = value.etag + fileProviderContentVersion = value.fileProviderContentVersion favorite = value.favorite fileId = value.fileId fileName = value.fileName diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift index deda497552fc4..f41010ef83325 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift @@ -611,6 +611,12 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable { /// which etag. Captured before any injected `uploadError` short-circuit. public var lastUploadIfMatchHeader: String? + /// Records the WebDAV `If` header the most recent upload call carried (nil if none). + public var lastUploadIfHeader: String? + + /// Lock information returned by lock and unlock requests. + public var lockUnlockResult: NKLock? + /// Handler to track enumerate calls public var enumerateCallHandler: ((String, EnumerateDepth, Bool, [String], Data?, Account, NKRequestOptions, @escaping (URLSessionTask) -> Void) -> Void)? @@ -794,6 +800,7 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable { remoteError: NKError ) { lastUploadIfMatchHeader = options.customHeader?["If-Match"] + lastUploadIfHeader = options.customHeader?["If"] if let uploadError { return (account.ncKitAccount, nil, nil, nil, 0, nil, uploadError) @@ -1300,8 +1307,11 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable { } item.locked = shouldLock + if shouldLock, let etag = lockUnlockResult?.etag { + item.versionIdentifier = etag + } - return nil + return lockUnlockResult } public func listingTrashAsync( diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift index 604417e10fe23..f95acb5ea60d9 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift @@ -2205,6 +2205,26 @@ final class FilesDatabaseManagerTests: NextcloudFileProviderKitTestCase { XCTAssertTrue(storedRotated.downloaded) } + func testAddItemMetadataPreservingLocalStateKeepsContentVersionForSameEtag() { + let account = Account(user: "test", id: "t", serverUrl: "https://example.com", password: "") + + var original = SendableItemMetadata(ocId: "item", fileName: "locked.txt", account: account) + original.etag = "etag-after-lock" + original.fileProviderContentVersion = "etag-before-lock" + Self.dbManager.addItemMetadata(original) + + var refreshed = SendableItemMetadata(ocId: "item", fileName: "locked.txt", account: account) + refreshed.etag = "etag-after-lock" + + let merged = Self.dbManager.addItemMetadataPreservingLocalState(refreshed) + + XCTAssertEqual(merged.fileProviderContentVersion, "etag-before-lock") + XCTAssertEqual( + Self.dbManager.itemMetadata(ocId: "item")?.fileProviderContentVersion, + "etag-before-lock" + ) + } + func testAddItemMetadataPreservingLocalStateFallbackDoesNotMergeWhenAlreadyDuplicated() throws { let account = Account(user: "test", id: "t", serverUrl: "https://example.com", password: "") let fileName = "duped.txt" diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift index 55d48bf723324..4b1839e7de8f3 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift @@ -792,6 +792,16 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { func testCreateLockFileTriggersRemoteLockInsteadOfUpload() async { let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) + remoteInterface.lockUnlockResult = NKLock( + owner: Self.account.id, + ownerEditor: "", + ownerType: .token, + ownerDisplayName: Self.account.username, + time: nil, + timeOut: nil, + token: "files_lock/test-token", + etag: "etag-after-lock" + ) // Setup remote folder and file let folderRemote = MockRemoteItem( @@ -820,6 +830,7 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { serverUrl: Self.account.serverUrl ) + targetRemote.parent = folderRemote folderRemote.children = [targetRemote] folderRemote.parent = rootItem rootItem.children = [folderRemote] @@ -835,6 +846,9 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { ocId: targetRemote.identifier, fileName: targetFileName, account: Self.account ) targetMetadata.serverUrl += "/folder" + targetMetadata.etag = "etag-before-lock" + targetMetadata.downloaded = true + targetMetadata.syncTime = Date(timeIntervalSince1970: 1) Self.dbManager.addItemMetadata(targetMetadata) // Construct the lock file metadata @@ -868,6 +882,57 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { XCTAssertNil(error) XCTAssertNotNil(Self.dbManager.itemMetadata(ocId: lockFileMetadata.ocId)) XCTAssertTrue(targetRemote.locked) + let lockedMetadata = Self.dbManager.itemMetadata(ocId: targetRemote.identifier) + XCTAssertEqual(lockedMetadata?.etag, "etag-after-lock") + XCTAssertEqual(lockedMetadata?.lockToken, "files_lock/test-token") + XCTAssertEqual( + lockedMetadata?.fileProviderContentVersion, + "etag-before-lock", + "A lock-only etag transition must preserve File Provider's content version." + ) + XCTAssertTrue( + Self.dbManager.pendingWorkingSetChanges(since: Date(timeIntervalSince1970: 2)).updated + .contains(where: { $0.ocId == targetRemote.identifier }), + "Recovering the lock token must queue the target item for a File Provider metadata refresh." + ) + + if let lockedMetadata { + let lockedItem = Item( + metadata: lockedMetadata, + parentItemIdentifier: .init(folderMetadata.ocId), + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager + ) + XCTAssertEqual(lockedItem.itemVersion.contentVersion, Data("etag-before-lock".utf8)) + } + + let targetRead = await Enumerator.readServerUrl( + targetRemote.remotePath, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager, + depth: .target, + log: FileProviderLogMock() + ) + XCTAssertEqual( + targetRead.metadatas?.first?.fileProviderContentVersion, + "etag-before-lock", + "Refreshing the locked target must keep the content version from before the lock." + ) + + let laterRead = await Enumerator.readServerUrl( + folderRemote.remotePath, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager, + depth: .targetAndDirectChildren, + log: FileProviderLogMock() + ) + XCTAssertFalse( + laterRead.changes?.createdAndUpdated.contains(where: { $0.ocId == targetRemote.identifier }) ?? true, + "A later enumeration must not report the owner's lock etag as a content update." + ) } func testCreateLockFileUnactionableWithoutCapabilities() async throws { diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift index ab5c246bdb839..fe75e8f43c43a 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift @@ -421,17 +421,10 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { ) XCTAssertNil(modifiedItem) - // On macOS < 26 the heuristic sends `If-Match` on every content upload - // (the item has a base etag), so a 412 is now read as a version conflict and - // returns a transient NSCocoaError. On macOS 26+ no `.failOnConflict` was - // requested, so no `If-Match` is sent and 412 keeps its stale-lock meaning. - if #available(macOS 26.0, *) { - XCTAssertEqual((error as? NSFileProviderError)?.code, .cannotSynchronize) - } else { - let nsError = error as NSError? - XCTAssertEqual(nsError?.domain, NSCocoaErrorDomain) - XCTAssertEqual(nsError?.code, NSFileWriteUnknownError) - } + // The lock token is the upload precondition, so the pre-lock base etag is not + // also sent as If-Match. A 412 therefore keeps its stale-lock meaning. + XCTAssertNil(remoteInterface.lastUploadIfMatchHeader) + XCTAssertEqual((error as? NSFileProviderError)?.code, .cannotSynchronize) let updatedMetadata = Self.dbManager.itemMetadata(ocId: itemMetadata.ocId) XCTAssertNil(updatedMetadata?.lockToken, "Stale lock token must be cleared on 412.") } @@ -488,6 +481,67 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { ) } + /// The server changes the etag while acquiring an exclusive token lock, but + /// File Provider correctly supplies the version from which the document was + /// opened. The token must be the only write precondition for that owner upload; + /// combining it with the pre-lock etag would reject the first save. + func testModifyWithLockTokenDoesNotPassPreLockIfMatch() async throws { + let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) + + var itemMetadata = remoteItem.toItemMetadata(account: Self.account) + itemMetadata.etag = "etag-after-lock" + itemMetadata.fileProviderContentVersion = "etag-before-lock" + itemMetadata.lockToken = "files_lock/test-token" + Self.dbManager.addItemMetadata(itemMetadata) + + let newContentsUrl = FileManager.default.temporaryDirectory + .appendingPathComponent("modify-locked-with-pre-lock-version") + try "Updated content".write(to: newContentsUrl, atomically: true, encoding: .utf8) + + let item = Item( + metadata: itemMetadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager + ) + let targetItem = Item( + metadata: itemMetadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager + ) + let baseVersion = NSFileProviderItemVersion( + contentVersion: Data("etag-before-lock".utf8), + metadataVersion: Data("etag-before-lock".utf8) + ) + let options: NSFileProviderModifyItemOptions = if #available(macOS 26.0, *) { + .failOnConflict + } else { + [] + } + + let (modifiedItem, error) = await item.modify( + itemTarget: targetItem, + baseVersion: baseVersion, + changedFields: [.contents, .contentModificationDate], + contents: newContentsUrl, + options: options, + dbManager: Self.dbManager + ) + + XCTAssertNil(error) + XCTAssertNotNil(modifiedItem) + XCTAssertNil(remoteInterface.lastUploadIfMatchHeader) + XCTAssertEqual( + remoteInterface.lastUploadIfHeader, + "<\(remoteItem.remotePath)> ()" + ) + XCTAssertEqual(modifiedItem?.metadata.fileProviderContentVersion, modifiedItem?.metadata.etag) + XCTAssertNotEqual(modifiedItem?.itemVersion.contentVersion, Data("etag-before-lock".utf8)) + } + /// macOS 26+: a 412 from the server while `If-Match` was sent means the /// server copy changed under us. With `.failOnConflict` the extension must /// return `.localVersionConflictingWithServer` so the system creates a conflict @@ -501,7 +555,6 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { remoteInterface.uploadError = NKError(statusCode: 412, fallbackDescription: "Precondition Failed") var itemMetadata = remoteItem.toItemMetadata(account: Self.account) - itemMetadata.lockToken = "opaquelocktoken:token" itemMetadata.uploaded = true itemMetadata.downloaded = true Self.dbManager.addItemMetadata(itemMetadata) @@ -538,7 +591,6 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { XCTAssertEqual(remoteInterface.lastUploadIfMatchHeader, "\"0\"") let updated = Self.dbManager.itemMetadata(ocId: itemMetadata.ocId) - XCTAssertNil(updated?.lockToken, "Lock token must be cleared on conflict.") XCTAssertNotEqual( updated?.status, Status.normal.rawValue, "A rejected upload must not be committed as a normal, synced item." @@ -558,7 +610,6 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { remoteInterface.uploadError = NKError(statusCode: 412, fallbackDescription: "Precondition Failed") var itemMetadata = remoteItem.toItemMetadata(account: Self.account) - itemMetadata.lockToken = "opaquelocktoken:token" itemMetadata.uploaded = true itemMetadata.downloaded = true Self.dbManager.addItemMetadata(itemMetadata) @@ -594,9 +645,6 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { XCTAssertEqual(nsError?.domain, NSCocoaErrorDomain) XCTAssertEqual(nsError?.code, NSFileWriteUnknownError) XCTAssertEqual(remoteInterface.lastUploadIfMatchHeader, "\"0\"") - - let updated = Self.dbManager.itemMetadata(ocId: itemMetadata.ocId) - XCTAssertNil(updated?.lockToken, "Lock token must be cleared on conflict.") } func testModifyWith423ClearsLockToken() async throws { diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemPropertyTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemPropertyTests.swift index b36b8f4223ab6..c1417f07d32f0 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemPropertyTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemPropertyTests.swift @@ -38,6 +38,59 @@ final class ItemPropertyTests: NextcloudFileProviderKitTestCase { XCTAssertEqual(item.contentType, UTType.text) } + func testItemVersionFallsBackToEtagWithoutStoredContentVersion() { + var metadata = + SendableItemMetadata(ocId: "test-id", fileName: "test.txt", account: Self.account) + metadata.etag = "test-etag" + + let item = Item( + metadata: metadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: MockRemoteInterface(account: Self.account), + dbManager: Self.dbManager + ) + + XCTAssertEqual(item.itemVersion.contentVersion, Data("test-etag".utf8)) + } + + func testRecoveringLockTokenWithSameEtagRefreshesCapabilitiesWithoutChangingContentVersion() { + var metadata = + SendableItemMetadata(ocId: "test-id", fileName: "test.txt", account: Self.account) + metadata.etag = "unchanged-lock-etag" + metadata.fileProviderContentVersion = "content-etag" + metadata.lock = true + metadata.ownerId = Self.account.id + metadata.lockOwner = Self.account.id + metadata.lockOwnerType = NKLockType.token.rawValue + metadata.lockTimeOut = Date().addingTimeInterval(3600) + metadata.lockToken = nil + + let itemWithoutToken = Item( + metadata: metadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: MockRemoteInterface(account: Self.account), + dbManager: Self.dbManager + ) + + metadata.lockToken = "files_lock/recovered-token" + let itemWithToken = Item( + metadata: metadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: MockRemoteInterface(account: Self.account), + dbManager: Self.dbManager + ) + + XCTAssertFalse(itemWithoutToken.capabilities.contains(.allowsWriting)) + XCTAssertTrue(itemWithToken.capabilities.contains(.allowsWriting)) + XCTAssertFalse(itemWithoutToken.fileSystemFlags.contains(.userWritable)) + XCTAssertTrue(itemWithToken.fileSystemFlags.contains(.userWritable)) + XCTAssertNotEqual(itemWithoutToken.itemVersion.metadataVersion, itemWithToken.itemVersion.metadataVersion) + XCTAssertEqual(itemWithoutToken.itemVersion.contentVersion, itemWithToken.itemVersion.contentVersion) + } + func testMetadataExtensionContentType() { var metadata = SendableItemMetadata(ocId: "test-id", fileName: "test.pdf", account: Self.account)