From 3417a8f14d1befac6fa94e750ea0794fdd1c6c6a Mon Sep 17 00:00:00 2001 From: OpenCow Date: Sun, 2 Aug 2026 13:58:57 +0200 Subject: [PATCH 1/2] feat: harden conflict resolution --- AGENTS.md | 14 + .../potassiumProviderFileProviderInfo.plist | 2 + PotassiumProviderCore/KDriveModels.swift | 135 +++++++- .../KDriveMutationCoordinator.swift | 244 ++++++++++++--- .../KDriveRemoteService.swift | 148 ++++++--- README.md | 21 +- doc/CONFLICTS.md | 193 ++++++------ doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md | 210 +++++++++++++ doc/FILE_LISTING_CACHING_IMPROVEMENTS.md | 16 +- doc/LISTING_AND_VERSIONING.md | 20 +- doc/MUTATIONS.md | 102 ++++--- potassiumProvider.xcodeproj/project.pbxproj | 4 +- .../xcshareddata/swiftpm/Package.resolved | 6 +- .../ProviderActivityTimelineRow.swift | 41 +++ .../FileProviderRuntime.swift | 35 +++ .../PotassiumFileProviderExtension.swift | 277 ++++++++++------- .../KDriveContextActionTests.swift | 16 +- .../KDriveMachineNamespaceResolverTests.swift | 14 +- .../KDriveMutationCoordinatorTests.swift | 288 ++++++++++++++---- .../KDrivePrivateDirectoryResolverTests.swift | 14 +- .../KnownFolderLifecycleTests.swift | 14 +- .../WorkingSetSyncTests.swift | 5 +- .../potassiumProviderTests.swift | 49 ++- 23 files changed, 1429 insertions(+), 439 deletions(-) create mode 100644 doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md diff --git a/AGENTS.md b/AGENTS.md index af7a57f..95fe12e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,6 +28,8 @@ potassiumProvider/ |-- potassiumProviderActions/ # File Provider UI contextual panels |-- potassiumProviderTests/ # Swift Testing unit tests |-- potassiumProviderUITests/ # XCTest UI automation tests +|-- doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md +| # Mission-critical conflict safety register |-- potassiumProvider.xcodeproj/ # Source of truth for targets, scheme, settings, | # and SwiftPM package pins `-- SynchronizingFilesUsingFileProviderExtensions/ @@ -157,6 +159,15 @@ app. - Treat `NSFileProvider` enumeration, sync anchors, item identifiers, progress, cancellation, and completion handlers as correctness-critical. +- Treat `doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md` as the mission-critical, + normative conflict safety register. Read it before changing File Provider + mutations, versions, kDrive conflict policy, error mapping, retry/staging, + reconciliation, cleanup, or recovery UI. +- Update `doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md` in the same change whenever a + truth-table decision, risk, recovery path, or supporting test changes. Update + its audit evidence and finding states, and add regression coverage for every + affected decision cell. An inaccurate or stale table is a release-blocking + data-safety defect. - Treat Apple's Replicated File Provider extension documentation as the source of truth for replicated File Provider implementation. Check it when planning changes in this area: @@ -184,6 +195,9 @@ app. - Keep `README.md` and `doc/` documentation up to date when changing behavior, architecture, public interfaces, persistence, File Provider lifecycle, kDrive API mapping, conflict handling, or validation commands. +- Conflict-related changes are not documentation-complete until + `doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md` has been reviewed and updated. Do not + defer this mission-critical maintenance to a follow-up. - Prefer updating the subject-specific document in `doc/` as part of the same change instead of leaving documentation follow-up work implicit. diff --git a/Config/potassiumProviderFileProviderInfo.plist b/Config/potassiumProviderFileProviderInfo.plist index d873767..b124902 100644 --- a/Config/potassiumProviderFileProviderInfo.plist +++ b/Config/potassiumProviderFileProviderInfo.plist @@ -24,6 +24,8 @@ NSExtensionFileProviderSupportsEnumeration + NSExtensionFileProviderSupportsFailingUploadOnConflict + NSExtensionFileProviderDocumentGroup group.net.weavee.potassiumProvider NSExtensionFileProviderActions diff --git a/PotassiumProviderCore/KDriveModels.swift b/PotassiumProviderCore/KDriveModels.swift index 58736e7..6616894 100644 --- a/PotassiumProviderCore/KDriveModels.swift +++ b/PotassiumProviderCore/KDriveModels.swift @@ -32,7 +32,9 @@ public struct KDriveRemoteItem: Codable, Equatable, Identifiable, Sendable { public let isFavorite: Bool? public let createdAt: Date? public let modifiedAt: Date + public let revisedAt: Date? public let updatedAt: Date + public let etag: String? public init( id: Int, @@ -47,7 +49,9 @@ public struct KDriveRemoteItem: Codable, Equatable, Identifiable, Sendable { isFavorite: Bool? = nil, createdAt: Date?, modifiedAt: Date, - updatedAt: Date + revisedAt: Date? = nil, + updatedAt: Date, + etag: String? = nil ) { self.id = id self.name = name @@ -61,7 +65,9 @@ public struct KDriveRemoteItem: Codable, Equatable, Identifiable, Sendable { self.isFavorite = isFavorite self.createdAt = createdAt self.modifiedAt = modifiedAt + self.revisedAt = revisedAt self.updatedAt = updatedAt + self.etag = etag } public var isDirectory: Bool { @@ -76,7 +82,12 @@ public struct KDriveRemoteItem: Codable, Equatable, Identifiable, Sendable { } public var contentVersion: Data { - Data(String(modifiedAt.timeIntervalSince1970).utf8) + KDriveItemContentVersion( + itemID: id, + etag: etag, + revisedAt: revisedAt ?? modifiedAt, + size: size + ).data } public var metadataVersion: Data { @@ -89,7 +100,91 @@ public struct KDriveRemoteItem: Codable, Equatable, Identifiable, Sendable { } } +public struct KDriveItemContentVersion: Equatable, Sendable { + private static let currentVersion = 2 + + public let itemID: Int? + public let etag: String? + public let revisedAt: Date + public let size: Int? + public let isLegacy: Bool + + public init(itemID: Int, etag: String?, revisedAt: Date, size: Int?) { + self.itemID = itemID + self.etag = etag?.trimmingCharacters(in: .whitespacesAndNewlines).nilIfEmpty + self.revisedAt = revisedAt + self.size = size + self.isLegacy = false + } + + public init?(data: Data) { + if let payload = try? JSONDecoder().decode(Payload.self, from: data), + payload.version == Self.currentVersion, + payload.itemID > 0 { + self.itemID = payload.itemID + self.etag = payload.etag?.trimmingCharacters(in: .whitespacesAndNewlines).nilIfEmpty + self.revisedAt = Date(timeIntervalSince1970: payload.revisedAt) + self.size = payload.size + self.isLegacy = false + return + } + + guard let rawValue = String(data: data, encoding: .utf8), + let timestamp = TimeInterval(rawValue) else { + return nil + } + self.itemID = nil + self.etag = nil + self.revisedAt = Date(timeIntervalSince1970: timestamp) + self.size = nil + self.isLegacy = true + } + + public var isAuthoritative: Bool { + isLegacy == false && itemID != nil && etag != nil + } + + public var data: Data { + let payload = Payload( + version: Self.currentVersion, + itemID: itemID ?? 0, + etag: etag, + revisedAt: revisedAt.timeIntervalSince1970, + size: size + ) + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + return (try? encoder.encode(payload)) ?? Data() + } + + public func authoritativelyMatches(_ item: KDriveRemoteItem) -> Bool { + guard isAuthoritative, + itemID == item.id, + let etag, + let remoteETag = item.etag?.trimmingCharacters(in: .whitespacesAndNewlines).nilIfEmpty else { + return false + } + return etag == remoteETag + } + + private struct Payload: Codable { + let version: Int + let itemID: Int + let etag: String? + let revisedAt: TimeInterval + let size: Int? + } +} + +private extension String { + var nilIfEmpty: String? { + isEmpty ? nil : self + } +} + public struct KDriveItemMetadataVersion: Equatable, Sendable { + private static let currentVersion = 2 + public let itemID: Int public let updatedAt: Date public let name: String @@ -103,9 +198,20 @@ public struct KDriveItemMetadataVersion: Equatable, Sendable { } public init?(data: Data) { - guard let rawValue = String(data: data, encoding: .utf8) else { - return nil + if let payload = try? JSONDecoder().decode(Payload.self, from: data), + payload.version == Self.currentVersion, + payload.itemID > 0, + payload.parentID > 0, + payload.name.isEmpty == false { + self.init( + itemID: payload.itemID, + updatedAt: Date(timeIntervalSince1970: payload.updatedAt), + name: payload.name, + parentID: payload.parentID + ) + return } + guard let rawValue = String(data: data, encoding: .utf8) else { return nil } self.init(rawValue: rawValue) } @@ -143,7 +249,16 @@ public struct KDriveItemMetadataVersion: Equatable, Sendable { } public var data: Data { - Data(rawValue.utf8) + let payload = Payload( + version: Self.currentVersion, + itemID: itemID, + updatedAt: updatedAt.timeIntervalSince1970, + name: name, + parentID: parentID + ) + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + return (try? encoder.encode(payload)) ?? Data() } public var rawValue: String { @@ -157,6 +272,14 @@ public struct KDriveItemMetadataVersion: Equatable, Sendable { public func matches(itemID expectedItemID: Int, name expectedName: String, parentID expectedParentID: Int) -> Bool { itemID == expectedItemID && name == expectedName && parentID == expectedParentID } + + private struct Payload: Codable { + let version: Int + let itemID: Int + let updatedAt: TimeInterval + let name: String + let parentID: Int + } } public struct KDriveItemPage: Equatable, Sendable { @@ -420,7 +543,7 @@ public enum KDriveVersionConflictResolver { } public static func contentMatches(baseVersion: Data, remoteItem: KDriveRemoteItem) -> Bool { - baseVersion == remoteItem.contentVersion + KDriveItemContentVersion(data: baseVersion)?.authoritativelyMatches(remoteItem) == true } public static func metadataMatches(baseVersion: Data, remoteItem: KDriveRemoteItem) -> Bool { diff --git a/PotassiumProviderCore/KDriveMutationCoordinator.swift b/PotassiumProviderCore/KDriveMutationCoordinator.swift index 5feabf6..93c9496 100644 --- a/PotassiumProviderCore/KDriveMutationCoordinator.swift +++ b/PotassiumProviderCore/KDriveMutationCoordinator.swift @@ -1,3 +1,4 @@ +import CryptoKit import Foundation public struct KDriveItemBaseVersion: Equatable, Sendable { @@ -12,13 +13,39 @@ public struct KDriveItemBaseVersion: Equatable, Sendable { public enum KDriveMutationConflictError: Error, LocalizedError, Sendable { case staleVersion(latestItem: KDriveRemoteItem) + case localContentConflict(latestItem: KDriveRemoteItem, stagedURL: URL) public var errorDescription: String? { - "The item changed on the server before the local mutation could be applied." + switch self { + case .staleVersion: + return "The item changed on the server before the local mutation could be applied." + case .localContentConflict: + return "The local upload conflicts with a newer server version." + } } public var recoverySuggestion: String? { - "Refresh the folder and retry the change." + switch self { + case .staleVersion: + return "Refresh the folder and retry the change." + case .localContentConflict: + return "Choose which version to keep; the local bytes have been retained for recovery." + } + } +} + +public enum KDriveMutationIdentity { + public static func contentHash(_ contents: Data) -> String { + "sha256:\(hexDigest(SHA256.hash(data: contents)))" + } + + public static func clientToken(_ components: [String]) -> String { + let input = Data(components.joined(separator: "\u{1f}").utf8) + return String(hexDigest(SHA256.hash(data: input)).prefix(32)) + } + + private static func hexDigest(_ digest: D) -> String where D.Element == UInt8 { + digest.map { String(format: "%02x", $0) }.joined() } } @@ -40,8 +67,15 @@ public struct KDriveAppGroupConflictContentStager: KDriveConflictContentStaging } let directoryURL = containerURL.appendingPathComponent("ConflictStaging", isDirectory: true) try FileManager.default.createDirectory(at: directoryURL, withIntermediateDirectories: true) + let recoveryKey = KDriveMutationIdentity.clientToken([ + itemIdentifier, + KDriveMutationIdentity.contentHash(contents), + ]) let fileURL = directoryURL - .appendingPathComponent("\(itemIdentifier)-\(UUID().uuidString)") + // Keep the provider-owned filename bounded and opaque. File Provider + // identifiers and user filenames can exceed filesystem component + // limits and must not leak into a private recovery path. + .appendingPathComponent(recoveryKey) .appendingPathExtension("upload") try contents.write(to: fileURL, options: [.atomic]) return fileURL @@ -131,24 +165,50 @@ public struct KDriveMutationCoordinator: Sendable { lastModifiedAt: Date?, transferProgress: (@Sendable (Progress) -> Void)? = nil ) async throws -> KDriveRemoteItem { + // File Provider owns the callback URL, so preserve our own deterministic + // copy before starting a create that may outlive this extension process. + let stagedURL = try await conflictStager.stageConflictContents( + contents, + itemIdentifier: "create-\(parentID)-\(fileName)" + ) + let contentHash = KDriveMutationIdentity.contentHash(contents) + let clientToken = KDriveMutationIdentity.clientToken([ + configuration.domainIdentifier, + "create", + String(parentID), + fileName, + contentHash, + ]) let operation = try remote.uploadFileOperation( driveID: configuration.driveID, parentID: parentID, fileName: fileName, contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: .version + conflictStrategy: .rename, + clientToken: clientToken, + contentHash: contentHash ) transferProgress?(operation.progress) - return try await operation.value + let item = try await operation.value + await conflictStager.removeStagedConflictContents(at: stagedURL) + return item } public func createDirectory(parentID: Int, name: String) async throws -> KDriveRemoteItem { - try await remote.createDirectory( - driveID: configuration.driveID, - parentID: parentID, - name: name - ) + do { + return try await remote.createDirectory( + driveID: configuration.driveID, + parentID: parentID, + name: name + ) + } catch where KDriveRemoteErrorClassifier.isNameCollision(error) { + return try await remote.createDirectory( + driveID: configuration.driveID, + parentID: parentID, + name: conflictName(for: name) + ) + } } public func replaceContents( @@ -158,30 +218,88 @@ public struct KDriveMutationCoordinator: Sendable { baseContentVersion: Data, contents: Data, lastModifiedAt: Date?, + failOnConflict: Bool = false, transferProgress: (@Sendable (Progress) -> Void)? = nil ) async throws -> KDriveContentMutationResult { + // Stage before any network preflight. If the process, network, or API fails, + // the exact local bytes remain available for a later retry or recovery. + let stagedURL = try await conflictStager.stageConflictContents(contents, itemIdentifier: itemIdentifier) + let contentHash = KDriveMutationIdentity.contentHash(contents) let latestItem = try await remote.item(driveID: configuration.driveID, fileID: fileID) - guard KDriveVersionConflictResolver.contentMatches(baseVersion: baseContentVersion, remoteItem: latestItem) else { + guard let baseVersion = KDriveItemContentVersion(data: baseContentVersion), + baseVersion.authoritativelyMatches(latestItem), + let expectedETag = baseVersion.etag else { + if failOnConflict { + throw KDriveMutationConflictError.localContentConflict( + latestItem: latestItem, + stagedURL: stagedURL + ) + } return try await uploadConflictCopy( itemIdentifier: itemIdentifier, localFilename: localFilename, latestItem: latestItem, + stagedURL: stagedURL, contents: contents, + contentHash: contentHash, lastModifiedAt: lastModifiedAt, transferProgress: transferProgress ) } + let clientToken = KDriveMutationIdentity.clientToken([ + configuration.domainIdentifier, + "replace", + String(fileID), + expectedETag, + contentHash, + ]) let operation = try remote.replaceFileOperation( driveID: configuration.driveID, - parentID: latestItem.parentID, - fileName: latestItem.name, + fileID: fileID, + expectedETag: expectedETag, + clientToken: clientToken, + contentHash: contentHash, contents: contents, lastModifiedAt: lastModifiedAt ) transferProgress?(operation.progress) - let replacedItem = try await operation.value - return .replaced(replacedItem) + do { + let replacedItem = try await operation.value + await conflictStager.removeStagedConflictContents(at: stagedURL) + return .replaced(replacedItem) + } catch { + guard KDriveRemoteErrorClassifier.isConditionalConflict(error) else { + await recordRetainedUploadFailure( + itemIdentifier: itemIdentifier, + localFilename: localFilename, + latestItem: latestItem, + stagedURL: stagedURL + ) + throw error + } + + let refreshedItem = (try? await remote.item( + driveID: configuration.driveID, + fileID: fileID + )) ?? latestItem + if failOnConflict { + throw KDriveMutationConflictError.localContentConflict( + latestItem: refreshedItem, + stagedURL: stagedURL + ) + } + return try await uploadConflictCopy( + itemIdentifier: itemIdentifier, + localFilename: localFilename, + latestItem: refreshedItem, + stagedURL: stagedURL, + contents: contents, + contentHash: contentHash, + lastModifiedAt: lastModifiedAt, + transferProgress: transferProgress + ) + } } public func renameItem( @@ -190,21 +308,19 @@ public struct KDriveMutationCoordinator: Sendable { name: String ) async throws -> KDriveRemoteItem { let latestItem = try await remote.item(driveID: configuration.driveID, fileID: fileID) - let desiredParentID = KDriveItemMetadataVersion(data: baseMetadataVersion)?.parentID ?? latestItem.parentID - guard let mutationState = KDriveVersionConflictResolver.metadataMutationState( - baseVersion: baseMetadataVersion, - remoteItem: latestItem, - desiredName: name, - desiredParentID: desiredParentID - ) else { - throw KDriveMutationConflictError.staleVersion(latestItem: latestItem) - } - - if mutationState == .desired { + if latestItem.name == name { return latestItem } - try await remote.renameItem(driveID: configuration.driveID, fileID: fileID, name: name) + do { + try await remote.renameItem(driveID: configuration.driveID, fileID: fileID, name: name) + } catch where KDriveRemoteErrorClassifier.isNameCollision(error) { + try await remote.renameItem( + driveID: configuration.driveID, + fileID: fileID, + name: conflictName(for: name) + ) + } return try await remote.item(driveID: configuration.driveID, fileID: fileID) } @@ -215,17 +331,9 @@ public struct KDriveMutationCoordinator: Sendable { name: String? ) async throws -> KDriveRemoteItem { let latestItem = try await remote.item(driveID: configuration.driveID, fileID: fileID) - let desiredName = name ?? KDriveItemMetadataVersion(data: baseMetadataVersion)?.name ?? latestItem.name - guard let mutationState = KDriveVersionConflictResolver.metadataMutationState( - baseVersion: baseMetadataVersion, - remoteItem: latestItem, - desiredName: desiredName, - desiredParentID: destinationParentID - ) else { - throw KDriveMutationConflictError.staleVersion(latestItem: latestItem) - } - - if mutationState == .desired { + let baseName = KDriveItemMetadataVersion(data: baseMetadataVersion)?.name + let desiredName = name ?? latestItem.name + if latestItem.parentID == destinationParentID, latestItem.name == desiredName { return latestItem } @@ -233,21 +341,23 @@ public struct KDriveMutationCoordinator: Sendable { driveID: configuration.driveID, fileID: fileID, destinationParentID: destinationParentID, - name: name + // Preserve an independent remote rename for a move-only local edit. + name: name == nil && baseName != latestItem.name ? nil : name + ) + return try await remote.item(driveID: configuration.driveID, fileID: fileID) + } + + public func updateModificationDate(fileID: Int, date: Date) async throws -> KDriveRemoteItem { + try await remote.updateModificationDate( + driveID: configuration.driveID, + fileID: fileID, + date: date ) return try await remote.item(driveID: configuration.driveID, fileID: fileID) } public func trashItem(fileID: Int, baseVersion: KDriveItemBaseVersion) async throws -> KDriveRemoteItem { let latestItem = try await remote.item(driveID: configuration.driveID, fileID: fileID) - guard KDriveVersionConflictResolver.itemVersionMatchesAllowingMetadataTimestampDrift( - contentVersion: baseVersion.contentVersion, - metadataVersion: baseVersion.metadataVersion, - remoteItem: latestItem - ) else { - throw KDriveMutationConflictError.staleVersion(latestItem: latestItem) - } - try await remote.trashItem(driveID: configuration.driveID, fileID: fileID) return latestItem } @@ -270,11 +380,12 @@ public struct KDriveMutationCoordinator: Sendable { itemIdentifier: String, localFilename: String, latestItem: KDriveRemoteItem, + stagedURL: URL, contents: Data, + contentHash: String, lastModifiedAt: Date?, transferProgress: (@Sendable (Progress) -> Void)? ) async throws -> KDriveContentMutationResult { - let stagedURL = try await conflictStager.stageConflictContents(contents, itemIdentifier: itemIdentifier) let detectedAt = conflictDate() let context = KDriveStaleContentConflictContext( detectedAt: detectedAt, @@ -293,13 +404,22 @@ public struct KDriveMutationCoordinator: Sendable { ) do { + let clientToken = KDriveMutationIdentity.clientToken([ + configuration.domainIdentifier, + "conflict-copy", + itemIdentifier, + latestItem.etag ?? "missing-etag", + contentHash, + ]) let operation = try remote.uploadFileOperation( driveID: configuration.driveID, parentID: latestItem.parentID, fileName: conflictFilename, contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: .rename + conflictStrategy: .rename, + clientToken: clientToken, + contentHash: contentHash ) transferProgress?(operation.progress) let conflictItem = try await operation.value @@ -311,4 +431,30 @@ public struct KDriveMutationCoordinator: Sendable { throw error } } + + private func recordRetainedUploadFailure( + itemIdentifier: String, + localFilename: String, + latestItem: KDriveRemoteItem, + stagedURL: URL + ) async { + let context = KDriveStaleContentConflictContext( + detectedAt: conflictDate(), + localItemIdentifier: itemIdentifier, + localFilename: localFilename, + latestItem: latestItem, + stagedURL: stagedURL + ) + await contentConflictObserver?(.started(context)) + await contentConflictObserver?(.failed(context, failedAt: conflictDate())) + } + + private func conflictName(for name: String) -> String { + KDriveConflictFilename.filename( + for: name, + deviceName: conflictDeviceName(), + date: conflictDate(), + timeZone: conflictTimeZone() + ) + } } diff --git a/PotassiumProviderCore/KDriveRemoteService.swift b/PotassiumProviderCore/KDriveRemoteService.swift index 2cc73c6..e999036 100644 --- a/PotassiumProviderCore/KDriveRemoteService.swift +++ b/PotassiumProviderCore/KDriveRemoteService.swift @@ -21,7 +21,9 @@ public protocol KDriveFileProviding: KDriveItemMetadataProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) async throws -> KDriveRemoteItem func uploadFileOperation( driveID: Int, @@ -29,25 +31,32 @@ public protocol KDriveFileProviding: KDriveItemMetadataProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) throws -> KDriveTransferOperation func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem func replaceFileOperation( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) throws -> KDriveTransferOperation func createDirectory(driveID: Int, parentID: Int, name: String) async throws -> KDriveRemoteItem func renameItem(driveID: Int, fileID: Int, name: String) async throws func moveItem(driveID: Int, fileID: Int, destinationParentID: Int, name: String?) async throws + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws func trashItem(driveID: Int, fileID: Int) async throws func deleteTrashedItem(driveID: Int, fileID: Int) async throws } @@ -100,7 +109,9 @@ public extension KDriveFileProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) throws -> KDriveTransferOperation { let progress = Progress(totalUnitCount: Int64(max(contents.count, 1))) return KDriveTransferOperation(progress: progress) { @@ -110,7 +121,9 @@ public extension KDriveFileProviding { fileName: fileName, contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: conflictStrategy + conflictStrategy: conflictStrategy, + clientToken: clientToken, + contentHash: contentHash ) progress.completedUnitCount = progress.totalUnitCount return item @@ -119,8 +132,10 @@ public extension KDriveFileProviding { func replaceFileOperation( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) throws -> KDriveTransferOperation { @@ -128,8 +143,10 @@ public extension KDriveFileProviding { return KDriveTransferOperation(progress: progress) { let item = try await replaceFile( driveID: driveID, - parentID: parentID, - fileName: fileName, + fileID: fileID, + expectedETag: expectedETag, + clientToken: clientToken, + contentHash: contentHash, contents: contents, lastModifiedAt: lastModifiedAt ) @@ -140,6 +157,7 @@ public extension KDriveFileProviding { } public enum KDriveUploadConflictStrategy: String, Sendable { + case error case version case rename } @@ -188,7 +206,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public func item(driveID: Int, fileID: Int) async throws -> KDriveRemoteItem { try await performNetworkOperation("item") { - try await service.getFile(driveId: driveID, fileId: fileID).data.remoteItem + try await service.getFile(driveId: driveID, fileId: fileID, with: "etag").data.remoteItem } } @@ -197,6 +215,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot let response = try await service.listDirectoryFiles( driveId: driveID, fileId: folderID, + with: "etag", options: ListKDriveDirectoryFilesOptions(cursor: cursor, limit: limit, orderBy: ["name"], order: "asc") ) return KDriveItemPage(items: response.data.map(\.remoteItem), nextCursor: response.cursor, hasMore: response.hasMore) @@ -248,6 +267,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot try await performNetworkOperation("listTrash") { let response = try await service.listTrashFiles( driveId: driveID, + with: "etag", options: ListKDriveTrashOptions(cursor: cursor, limit: limit, orderBy: ["name"], order: "asc") ) return KDriveItemPage(items: response.data.map(\.remoteItem), nextCursor: response.cursor, hasMore: response.hasMore) @@ -256,10 +276,10 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public func listWorkingSetRelevantItems(driveID: Int, latestLimit: Int) async throws -> [KDriveRemoteItem] { try await performNetworkOperation("listWorkingSetRelevantItems") { - let latest = try await service.listLastModifiedFiles(driveId: driveID, limit: latestLimit).data - let favorites = try await service.listFavoriteFiles(driveId: driveID, limit: latestLimit).data - let myShared = try await service.listMySharedFiles(driveId: driveID, limit: latestLimit).data - let sharedWithMe = try await service.listSharedWithMeFiles(driveId: driveID, limit: latestLimit).data + let latest = try await service.listLastModifiedFiles(driveId: driveID, with: "etag", limit: latestLimit).data + let favorites = try await service.listFavoriteFiles(driveId: driveID, with: "etag", limit: latestLimit).data + let myShared = try await service.listMySharedFiles(driveId: driveID, with: "etag", limit: latestLimit).data + let sharedWithMe = try await service.listSharedWithMeFiles(driveId: driveID, with: "etag", limit: latestLimit).data var itemsByID: [Int: KDriveRemoteItem] = [:] for item in latest + favorites + myShared + sharedWithMe { itemsByID[item.id] = item.remoteItem @@ -280,6 +300,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot return try await performNetworkOperation("listPartialActivities") { let response = try await service.listPartialFileActivities( driveId: driveID, + with: "file,file.etag", options: ListKDrivePartialFileActivitiesOptions( actions: [ "file_create", "file_delete", "file_trash", "file_restore", @@ -339,7 +360,9 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String? = nil, + contentHash: String? = nil ) async throws -> KDriveRemoteItem { try await uploadFileOperation( driveID: driveID, @@ -347,7 +370,9 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot fileName: fileName, contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: conflictStrategy + conflictStrategy: conflictStrategy, + clientToken: clientToken, + contentHash: contentHash ).value } @@ -357,16 +382,21 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String? = nil, + contentHash: String? = nil ) throws -> KDriveTransferOperation { let operation = try service.uploadFile( driveId: driveID, data: contents, options: UploadKDriveFileOptions( + with: "etag", + clientToken: clientToken, conflict: conflictStrategy.rawValue, directoryId: parentID, fileName: fileName, - lastModifiedAt: lastModifiedAt.map(Self.unixTimestamp) + lastModifiedAt: lastModifiedAt.map(Self.unixTimestamp), + totalChunkHash: contentHash ) ) return KDriveTransferOperation( @@ -382,15 +412,19 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem { try await replaceFileOperation( driveID: driveID, - parentID: parentID, - fileName: fileName, + fileID: fileID, + expectedETag: expectedETag, + clientToken: clientToken, + contentHash: contentHash, contents: contents, lastModifiedAt: lastModifiedAt ).value @@ -398,8 +432,10 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public func replaceFileOperation( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) throws -> KDriveTransferOperation { @@ -407,10 +443,12 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot driveId: driveID, data: contents, options: UploadKDriveFileOptions( - conflict: KDriveUploadConflictStrategy.version.rawValue, - directoryId: parentID, - fileName: fileName, - lastModifiedAt: lastModifiedAt.map(Self.unixTimestamp) + with: "etag", + ifMatch: expectedETag, + clientToken: clientToken, + fileId: fileID, + lastModifiedAt: lastModifiedAt.map(Self.unixTimestamp), + totalChunkHash: contentHash ) ) return KDriveTransferOperation( @@ -451,6 +489,16 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot } } + public func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { + _ = try await performNetworkOperation("updateModificationDate") { + try await service.updateFileLastModified( + driveId: driveID, + fileId: fileID, + lastModifiedAt: Self.unixTimestamp(date) + ) + } + } + public func trashItem(driveID: Int, fileID: Int) async throws { _ = try await performNetworkOperation("trashItem") { try await service.trashFileV2(driveId: driveID, fileId: fileID) @@ -511,7 +559,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot try await service.getFileShareLink(driveId: driveID, fileId: fileID).data ) } - } catch APIClientError.unacceptableStatusCode(let statusCode, _) where statusCode == 404 { + } catch APIClientError.unacceptableStatusCode(let statusCode, _, _) where statusCode == 404 { return nil } } @@ -700,7 +748,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public enum KDriveRemoteErrorClassifier { public static func apiRejection(from error: Error) -> KDriveRemoteAPIRejection? { - guard case let APIClientError.unacceptableStatusCode(statusCode, body) = error else { + guard case let APIClientError.unacceptableStatusCode(statusCode, body, _) = error else { return nil } @@ -708,13 +756,41 @@ public enum KDriveRemoteErrorClassifier { } public static func isInvalidCursor(_ error: Error) -> Bool { - guard case let APIClientError.unacceptableStatusCode(_, body) = error else { + guard case let APIClientError.unacceptableStatusCode(_, body, _) = error else { return false } let lowercasedBody = body.lowercased() return lowercasedBody.contains("invalid") && lowercasedBody.contains("cursor") } + + /// Infomaniak documents `If-Match` support for uploads but does not specify + /// one exclusive rejection status. Accept both standard conflict responses. + public static func isConditionalConflict(_ error: Error) -> Bool { + guard let rejection = apiRejection(from: error) else { return false } + return rejection.statusCode == 409 || rejection.statusCode == 412 + } + + public static func isNotFound(_ error: Error) -> Bool { + apiRejection(from: error)?.statusCode == 404 + } + + public static func isNameCollision(_ error: Error) -> Bool { + guard let rejection = apiRejection(from: error) else { + return false + } + // A conflict response is unambiguous in the create/rename call sites + // that use this classifier. Do not make safe automatic renaming depend + // on the server returning a particular localized response body. + if rejection.statusCode == 409 { + return true + } + guard rejection.statusCode == 422 else { return false } + let body = rejection.responseBody.lowercased() + return body.contains("collision") + || body.contains("already exists") + || body.contains("name") + } } public struct KDriveRemoteAPIRejection: Equatable, Sendable { @@ -801,7 +877,9 @@ extension KDriveFileItem { isFavorite: isFavorite, createdAt: createdAt.map { Date(timeIntervalSince1970: TimeInterval($0)) }, modifiedAt: Date(timeIntervalSince1970: TimeInterval(lastModifiedAt)), - updatedAt: Date(timeIntervalSince1970: TimeInterval(updatedAt)) + revisedAt: revisedAt.map { Date(timeIntervalSince1970: TimeInterval($0)) }, + updatedAt: Date(timeIntervalSince1970: TimeInterval(updatedAt)), + etag: etag ) } } diff --git a/README.md b/README.md index 6edca12..2609aa2 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,11 @@ data. to potassiumChannel service calls and visible kDrive endpoints. - [Mutations](doc/MUTATIONS.md): create, upload, replace, rename, move, trash, delete, server-authoritative returns, and later reconciliation. +- [Conflict Resolution Truth Table And Safety Register](doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md): + mission-critical audited decisions, data-loss and soft-lock findings, user + recovery limits, and the mandatory maintenance procedure. - [Conflicts](doc/CONFLICTS.md): conflict cases, current resolution behavior, - risks, and safer future direction. + design context, risks, and safer future direction. - [File Provider Cleanup](doc/FILE_PROVIDER_CLEANUP.md): local development uninstall script, reset modes, stale registration repair, and safety boundary. - [Testing And Development](doc/TESTING_AND_DEVELOPMENT.md): schemes, @@ -139,8 +142,20 @@ local Xcode requires a more specific variant. - Do not commit bearer tokens, refresh tokens, account identifiers, private links, or user data. -- The current conflict handling delegates many decisions to kDrive. Read - [Conflicts](doc/CONFLICTS.md) before relying on it for important files. +- Conflict handling is mission-critical. Read the + [Conflict Resolution Truth Table And Safety Register](doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md) + before relying on the provider for important files. Any change to conflict + detection, mutation ordering, server conflict policy, retry/error behavior, + or user recovery must update that file in the same change; a stale table is a + release-blocking data-safety defect. +- The current conflict handling still delegates some decisions to kDrive. Read + [Conflicts](doc/CONFLICTS.md) for the broader design context. +- File creates stage bytes before their first network send. Existing-file + content uploads use kDrive ETags with `If-Match`, stage local bytes before + preflight, and preserve stale/raced edits as visible renamed copies unless + File Provider explicitly requests fail-on-conflict. Combined + move/rename/content/trash callbacks are applied in order; unapplied metadata + is returned as still pending. - SQLite snapshots cache metadata only. File contents and thumbnails are not stored there. - On macOS 15 or later, Desktop & Documents sync is an explicit, experimental diff --git a/doc/CONFLICTS.md b/doc/CONFLICTS.md index 272e565..86af764 100644 --- a/doc/CONFLICTS.md +++ b/doc/CONFLICTS.md @@ -1,14 +1,18 @@ # Conflict Cases And Resolution -This document is the source of truth for conflict behavior in -`potassiumProvider`. It describes the cases the File Provider extension can -encounter today, how the current implementation resolves or blocks them, and the -safe direction for future conflict work. +This document provides the design narrative for conflict behavior in +`potassiumProvider`. The mission-critical, normative decision matrix and open +safety findings are maintained in +[Conflict Resolution Truth Table And Safety Register](CONFLICT_RESOLUTION_TRUTH_TABLE.md). +If these documents disagree, the audited truth table takes precedence and the +inconsistency must be corrected. The provider is still mostly server-authoritative for final metadata: create, replace, move, rename, trash, and delete requests are sent to kDrive, and server -responses are returned to File Provider. The important guardrail now in place is -that stale local mutations are detected before the server is changed. +responses are returned to File Provider. Content replacement is guarded by a +server ETag and `If-Match`; metadata follows the documented local-intent policy; +permanent delete remains preflight-only because no conditional delete is +documented by Infomaniak. The listing cache introduced by `KDriveSnapshotSQLiteStore` is a metadata cache. It helps enumerate folders and diff server changes, but it is not a full sync @@ -21,6 +25,7 @@ used to resolve clickable user-visible URLs. Related docs: +- [Conflict Resolution Truth Table And Safety Register](CONFLICT_RESOLUTION_TRUTH_TABLE.md) - [File Provider Lifecycle](FILE_PROVIDER_LIFECYCLE.md) - [Listing And Versioning](LISTING_AND_VERSIONING.md) - [Mutations](MUTATIONS.md) @@ -31,10 +36,10 @@ Related docs: | Category | Meaning | Current examples | | --- | --- | --- | | Preserved both | Keep the remote item unchanged and save the local work as a separate item. | Stale content replacement uploads a renamed conflict copy. | -| Blocked/retryable | Refuse a stale local mutation before changing kDrive, then let File Provider refresh and retry. | Stale rename, move, trash, and permanent delete return `.cannotSynchronize`. | -| Delegated to kDrive | Send the operation with the current kDrive conflict flag and trust the server result. | New file upload uses `conflict=version`; move uses `conflict=rename`. | +| Blocked/retryable | Refuse a mutation before changing kDrive, retain local bytes where applicable, then let File Provider refresh and retry. | `.failOnConflict`, transient upload failures, and stale permanent delete. | +| Automatic local-intent merge | Apply the user's same-field intent to the latest stable item ID and preserve independent remote fields. | Rename wins the name field; move-only preserves a remote rename; move uses `conflict=rename`. | | Fail-closed | Treat ambiguous listing or cursor state as unsafe and stop before saving a bad snapshot. | Repeated cursors, missing continuation cursors, and unknown advanced actions throw. | -| Unresolved/future work | A known gap that needs durable local state, stronger server tokens, or explicit policy. | Folder create collisions, operation replay after timeout, and failed conflict-copy recovery. | +| Unresolved/future work | A known gap that needs a provider-owned retry schedule or a missing server primitive. | Recovered folders, unsupported metadata, large upload sessions, and conditional permanent delete. | Audit states stored in SQLite: @@ -75,11 +80,12 @@ Audit states stored in SQLite: The implementation follows these practical rules: 1. The server is the source of truth for item identity and final metadata. -2. Existing-item mutations fetch latest metadata before changing the server. -3. File Provider base versions are compared with latest remote versions. -4. Stale content replacement preserves both by uploading a renamed conflict copy. -5. Stale rename, move, trash, and permanent delete are blocked before server - mutation. +2. Local content bytes are staged before existing-item network preflight. +3. Content base versions compare stable item ID plus authoritative ETag. +4. Matching content replacement uses `file_id` plus `If-Match`; stale or raced + content preserves both unless `.failOnConflict` was requested. +5. Rename, move, and trash apply local intent; permanent delete requires an + unchanged strong base and returns `.deletionRejected` otherwise. 6. Later enumeration or change sync updates the SQLite snapshot from kDrive state. 7. Conflict-sensitive mutation decisions are covered by deterministic unit tests @@ -91,20 +97,22 @@ This is a focused conflict-safety pass, not the full sync-database redesign. `FileProviderItem` derives versions from kDrive metadata: -- `contentVersion`: `modifiedAt.timeIntervalSince1970` -- `metadataVersion`: `id`, `updatedAt`, `name`, and `parentID` +- `contentVersion`: versioned JSON containing item ID, ETag, `revisedAt`, and + size. Item ID plus ETag are authoritative; legacy timestamps fail closed. +- `metadataVersion`: versioned JSON containing ID, `updatedAt`, name, and parent + ID, with legacy decoding for rolling upgrades. Before mutating an existing item, the extension fetches latest metadata with `item(driveID:fileID:)` and compares the relevant base version: -- content replace checks content version -- rename and move parse metadata version and compare item ID, name, and parent, - while tolerating `updatedAt`-only drift -- trash and permanent delete keep content checks strict and tolerate - `updatedAt`-only metadata drift when item ID, name, and parent still match +- content replace checks item ID plus ETag and sends the ETag as `If-Match` +- rename and move apply the requested local fields to the latest stable item ID +- trash applies local intent because trash is recoverable +- permanent delete keeps content and metadata checks strict -These are timestamp-based conflict tokens. A future implementation should prefer -a kDrive revision, etag, checksum, or version ID if one is available. +Infomaniak documents `with=etag` on file/detail/listing routes and `If-Match` on +direct upload. The API does not document conditional rename, move, trash, or +permanent delete. ## Conflict Matrix @@ -112,21 +120,21 @@ a kDrive revision, etag, checksum, or version ID if one is available. | Case | Current behavior | Resolution category | Gap or safer direction | | --- | --- | --- | --- | -| New file vs existing file with same name | `createItem(...)` uploads with `KDriveUploadConflictStrategy.version`; kDrive decides whether to version, reject, or otherwise resolve. | Delegated to kDrive | Decide whether this provider should prefer preserve-both behavior with `conflict=rename` for local creates. | -| New folder vs existing folder with same name | `createDirectory(...)` is sent without a provider-side sibling preflight or explicit app-local conflict policy. | Delegated to kDrive | Define whether same-name folders should merge, fail with collision, or create a renamed folder. | -| New file vs existing folder, or new folder vs existing file | No provider-side file-versus-folder preflight exists today. | Delegated to kDrive | Add explicit type-aware collision handling and map rejected collisions to File Provider errors when possible. | +| New file vs existing file with same name | Upload uses `conflict=rename`, SHA-256, and deterministic `client_token`; both remain visible. | Preserved both automatically | Add identity reconciliation for `.mayAlreadyExist` to avoid safe but unnecessary duplicates. | +| New folder vs existing folder with same name | A recognized 409/422 collision retries with a conflict name. | Preserved both automatically | Live-validate every kDrive collision response shape; unrecognized rejection remains an error. | +| New file vs existing folder, or new folder vs existing file | File upload uses rename policy; directory collision retry is type-agnostic. | Preserved both or retryable error | Add explicit type-aware preflight for clearer results. | | Case-only collisions, such as `Report.txt` vs `report.txt` | No case-folded sibling check exists today. Behavior depends on kDrive and the local platform view. | Unresolved/future work | Add case-normalized collision detection before create, rename, and move. | -| Local create retried after server success but local reply failed | There is no idempotency key or pending-operation journal, so a retry can create duplicates or extra versions. | Unresolved/future work | Store pending operations durably with local operation IDs and reconcile by server item ID. | +| Local create retried after server success but local reply failed | File creates reuse a deterministic `client_token` and content hash. Directory create has no equivalent token. | Idempotent file retry; directory gap | Add provider-owned identity reconciliation and a directory replay primitive. | ### Content Modifications | Case | Current behavior | Resolution category | Gap or safer direction | | --- | --- | --- | --- | -| Local content edit vs unchanged remote content | The extension calls `replaceFile(...)`; replace uses kDrive upload with `fileId` and `conflict=version`. | Delegated to kDrive after base-version match | Prefer an authoritative server revision token over timestamps. | -| Local content edit vs remote content edit | The extension stages local bytes in app-group `ConflictStaging`, uploads a renamed conflict copy with `conflict=rename`, returns that item, and leaves the original untouched. | Preserved both | Add a durable retry/recovery workflow for staged conflict bytes. | -| Local content edit vs remote rename or move | Content base version may still match, so the provider replaces the latest item by stable file ID and uses the latest remote parent if a conflict copy is needed. | Delegated to kDrive or preserved both | Decide whether content edits should also check metadata drift when the user-visible path changed. | -| Local content edit vs remote trash or delete | Fetching latest metadata can fail, or the upload can fail if the parent/item is no longer valid. | Delegated to kDrive/error mapping | Stage all upload bytes before network sends and recover parent-deleted cases explicitly. | -| Failed conflict-copy upload | Staged bytes remain in `ConflictStaging`, but nothing automatically retries or surfaces them. | Unresolved/future work | Add a pending-operation table and a user-visible recovery path. | +| Local content edit vs unchanged remote content | Bytes are staged first, the current `(itemID, ETag)` is checked, then upload replaces by `file_id` with `If-Match`, SHA-256, and an idempotency token. | Automatic conditional replace | None for the direct-upload path; large upload sessions still need equivalent integration. | +| Local content edit vs remote content edit | The provider uploads a renamed conflict copy with `conflict=rename`, returns that item, and leaves the original untouched. | Preserved both automatically | The user may still need to compare or merge the visible files. | +| Local content edit vs remote rename or move | Replacement addresses the stable file ID. Metadata changes compose before content, and a move-only edit preserves an independent remote rename. | Automatic local-intent merge | kDrive does not document conditional rename/move, so same-field metadata is policy-based rather than transactional. | +| Local content edit vs remote trash or delete | Bytes are staged before lookup. A missing item or failed lookup returns an error without deleting the recovery copy. | Retryable with staged safety copy | A failure before conflict indexing can leave an unindexed staged file; provider-owned retry scheduling remains future work. | +| Failed conflict-copy upload | The deterministic staged copy remains; File Provider can retry, and Activities can reveal/export indexed copies. | Retryable plus user recovery | Add a provider-owned retry schedule and explicit “Retry now” action. | Conflict names are generated by `KDriveConflictFilename`, for example: @@ -147,12 +155,14 @@ Conflict-copy success is recorded as `automaticallyResolved` with | Case | Current behavior | Resolution category | Gap or safer direction | | --- | --- | --- | --- | -| Local rename vs unchanged remote metadata or `updatedAt`-only drift | The extension calls `renameItem(...)`, fetches the item again, and returns server metadata. | Delegated to kDrive after semantic base-version match | Return `NSFileProviderError.filenameCollision` when a same-parent name collision is locally detectable. | +| Local rename vs unchanged remote metadata or `updatedAt`-only drift | The extension calls `renameItem(...)`, fetches the item again, and returns server metadata. | Automatic local-intent merge | Keep collision response classification covered as the API evolves. | | Retried local rename already reflected on the server | The extension treats the desired final name and parent as success and returns latest metadata without another server rename. | Idempotent success | Keep this limited to exact desired final state. | -| Local move vs unchanged remote metadata or `updatedAt`-only drift | The extension calls `moveItem(...)`; move passes kDrive `conflict=rename`. | Delegated to kDrive after semantic base-version match | Document and test exact kDrive rename result once the API behavior is confirmed. | +| Local move vs unchanged remote metadata or `updatedAt`-only drift | The extension calls `moveItem(...)`; move passes kDrive `conflict=rename`. | Automatic local-intent merge | Document and live-test exact server-selected rename results. | | Retried local move already reflected on the server | The extension treats the desired final parent and optional name as success and returns latest metadata without another server move. | Idempotent success | Keep this limited to exact desired final state. | -| Local rename or move vs remote rename, move, or meaningful metadata change | The latest name or parent differs from both the base state and requested final state, so the extension does not mutate the server and returns `.cannotSynchronize` with a refresh/retry suggestion. | Blocked/retryable | Consider a richer recoverable error when the platform supports it. | -| Rename or move into an existing sibling name | Move delegates to kDrive with `conflict=rename`; rename has no provider-side sibling preflight. | Delegated to kDrive | Add local sibling lookup and collision mapping before mutating. | +| Local rename vs remote rename or move | The requested local name is applied to the latest stable item ID; an independent remote parent move is preserved. | Automatic local-intent merge | The remote same-field name loses by explicit policy. | +| Move-only vs remote rename | The provider passes no name, so the remote rename is preserved while the local destination is applied. | Automatic field merge | Keep regression coverage for independent field composition. | +| Combined move+rename vs remote metadata | The requested local parent and name are applied to the latest stable item ID. | Automatic local-intent merge | Same-field remote metadata loses by explicit policy. | +| Rename or move into an existing sibling name | Move delegates to kDrive with `conflict=rename`; recognized rename collision responses retry with a unique conflict name. | Preserved both names automatically | Add case-folded/type-aware sibling lookup for clearer preflight behavior. | | Rename swap, such as `a -> b` while `b -> a` | No provider-side bounce-rename strategy exists. | Unresolved/future work | Apple sample-style temporary bounce names can preserve both operations during swaps. | | Move into a deleted or stale parent | Destination parent resolution may fail or kDrive may reject the move. | Delegated to kDrive/error mapping | Treat parent-deleted paths as recovery cases, especially when file bytes are involved. | | Moving a parent while child changes are pending | There is no explicit child-sync barrier. | Unresolved/future work | Consider a File Provider barrier similar to the Apple sample's `waitForChanges(below:)` pattern. | @@ -164,12 +174,12 @@ tests or future support work: | Move shape | Current support | Follow-up support to remember | | --- | --- | --- | -| Move one item from parent `X` to parent `Y` while latest remote metadata still has the base name and parent, except `updatedAt` drift | Supported by semantic metadata matching. | Keep regression coverage for `updatedAt`-only drift. | +| Move one item from parent `X` to parent `Y` while latest remote metadata still has the base name and parent, except `updatedAt` drift | Supported by stable-ID local intent. | Keep regression coverage for `updatedAt`-only drift. | | Move several freshly created or uploaded folders into a new folder | Supported for folder `updatedAt` drift; successful mutations invalidate affected parent snapshots and signal File Provider containers. | Add a durable pending-operation journal if child uploads and parent moves need ordering across extension restarts. | | File Provider retries a move that the server already applied | Supported as idempotent success when latest remote name and parent exactly match the requested final state. | Keep the success condition exact so unrelated remote moves do not pass. | -| Move and rename in one operation | Supported when latest remote metadata is still at the base state or exactly at the requested final state. | Add local sibling collision checks before the server mutation. | -| Move requested but source is already in the same parent with the same name | Not special-cased as a provider-side no-op. | Return success without a server move when latest metadata already equals both base and requested final state. | -| Remote renamed or moved the source somewhere else first | Blocked before server mutation. | Surface richer user-facing conflict details when File Provider supports them. | +| Move and rename in one operation | Supported; both local same-field intents apply to the latest stable item ID. | Add case-folded/type-aware sibling collision checks before the server mutation. | +| Move requested but source is already in the requested parent with the requested name | Returned as idempotent success without another mutation. | Keep the desired-state check exact. | +| Remote renamed or moved the source somewhere else first | Local same-field intent wins; move-only preserves an independent remote rename. | Keep this explicit policy covered by tests. | | Destination parent was deleted, trashed, or is otherwise unavailable | Parent resolution or the kDrive move call fails through normal error mapping. | Add parent-deleted recovery policy, especially for folders with pending child changes. | | Move into an occupied sibling name, including case-only collisions | Delegated to kDrive for move with `conflict=rename`; rename has no provider-side sibling preflight. | Define local filename collision behavior for exact, case-folded, file/folder, and folder/folder collisions. | | Move a folder while children below it still have pending uploads or modifications | `updatedAt` drift from child activity is tolerated, but there is no explicit subtree barrier. | Add a pending-operation barrier before moving parent folders when children are still syncing. | @@ -180,19 +190,15 @@ tests or future support work: | Case | Current behavior | Resolution category | Gap or safer direction | | --- | --- | --- | --- | -| Trash vs unchanged remote item or `updatedAt`-only metadata drift | The extension calls `trashItem(...)` after content matches and item ID, name, and parent still match. | Delegated to kDrive after semantic base-version match | Keep destructive operations guarded by both content and metadata checks. | -| Trash vs remote edit, rename, or move | The content changed, or latest name/parent differs from the base state, so the extension does not mutate the server and returns `.cannotSynchronize`. | Blocked/retryable | A future UI can explain which remote change blocked the trash. | +| Trash vs unchanged remote item or `updatedAt`-only metadata drift | The extension calls `trashItem(...)` after applying other fields requested in the same callback. | Automatic local-intent mutation | Trash remains reversible. | +| Trash vs remote edit, rename, or move | The provider applies local trash intent to the latest stable item ID. | Automatic local-intent mutation | Restore from trash if the local intent was wrong. | | Permanent delete of trashed item vs unchanged remote item or `updatedAt`-only metadata drift | The extension calls `deleteTrashedItem(...)` after content matches and item ID, name, and parent still match. | Delegated to kDrive after semantic base-version match | Keep permanent delete restricted to trash items. | -| Permanent delete vs remote edit, restore, rename, or move | The content changed, latest name/parent differs from the base state, or the latest metadata fetch no longer matches, so the operation is blocked or mapped through `providerError(...)`. | Blocked/retryable or delegated error mapping | Map server rejection to File Provider's rejected-deletion shape where available. | -| Delete-delete or item already gone remotely | kDrive's response is mapped through `providerError(...)`; there is no local special-case success policy. | Delegated to kDrive/error mapping | Decide whether already-gone deletes should be treated as success for idempotency. | +| Permanent delete vs remote edit, restore, rename, or move | The provider rejects deletion before mutation and returns `.deletionRejected` with the latest item. | Rejected safely for user review | Infomaniak still documents no conditional delete, so a post-preflight race remains. | +| Delete-delete or item already gone remotely | A 404 latest lookup is treated as idempotent success. | Idempotent success | Keep this limited to authoritative not-found responses. | -On the current iOS File Provider target, stale destructive mutations are returned -as `.cannotSynchronize` with a recovery suggestion to refresh and retry. This is -the platform-compatible form of the intended "version no longer available" -behavior. - -Stale rename, move, trash, and permanent delete attempts are recorded as -`blockedRetryable` with `blockedBeforeServerMutation`. +Stale permanent delete is the destructive case that deliberately needs user +review: `.deletionRejected` lets File Provider recreate the latest server item. +Rename, move, and trash follow the automatic local-intent policy above. ## Activities Tab @@ -204,6 +210,10 @@ The app has an Activities tab backed by `Snapshots.sqlite3`. - Entries are grouped by day and use compact summaries. Expanding an entry reveals conflict state, diagnostics, recovery guidance, identifiers, copy, and item actions without making every row expensive to render. +- Unresolved staged uploads older than 24 hours display **Needs Attention**. + Indexed staged bytes can be revealed in Finder on macOS or exported through + the share sheet on iOS and visionOS; paths are accepted only from the app + group's `ConflictStaging` directory. - File links are resolved from stored domain and item identifiers through `NSFileProviderManager.getUserVisibleURL(for:)` only after the user selects **Open in Finder** on macOS or **Open in Files** on iOS and visionOS. @@ -264,11 +274,11 @@ listing loops return `.cannotSynchronize`. | Case | Current behavior | Resolution category | Gap or safer direction | | --- | --- | --- | --- | -| Network timeout before knowing whether the server succeeded | There is no provider-layer idempotency key, so retry behavior is mostly controlled by kDrive. | Unresolved/future work | Add a durable pending-operation journal with idempotency metadata. | +| Network timeout before knowing whether the server succeeded | Direct file create/replace/conflict-copy requests reuse deterministic tokens and hashes; staged content remains after failures. | Idempotent retry at API boundary | Add a provider-owned pending scheduler and equivalent large-upload-session support. | | OAuth missing or expired beyond refresh | Runtime loading fails and maps to a File Provider authentication error. | Blocked/retryable | Keep secrets out of logs and surface reauthentication through app-owned flows. | | Server unreachable | URL/network errors map to `.serverUnreachable`. | Blocked/retryable | Preserve retryability and avoid committing local snapshot state during failures. | | Insufficient quota | kDrive errors should map to provider errors when classified by the API layer. | Blocked/retryable | Ensure quota failures are surfaced as `.insufficientQuota` when possible. | -| File bytes disappear before upload completes | Only stale content conflict uploads are staged before upload. Normal creates/replaces read bytes and send them directly. | Unresolved/future work | Store local upload bytes durably before every network send. | +| File Provider callback URL disappears before upload completes | File creates and existing-item replacement copy the bytes into deterministic app-group staging before the network request. | Provider-owned bytes retained | Index preflight/create failures in a complete pending-operation journal. | ## Lessons From Other Projects @@ -287,9 +297,10 @@ without claiming feature parity. author and time information. - Apple's local `SynchronizingFilesUsingFileProviderExtensions/` sample shows File Provider-specific tools that this project does not yet implement: - `filenameCollision` errors, rejected deletion errors, conflict-version listing - and keep-version actions, bounce renames for swaps, and barriers before moving - parents with children still syncing. + conflict-version listing and keep-version actions, bounce renames for swaps, + and barriers before moving parents with children still syncing. This provider + now handles common collisions automatically and uses rejected deletion for a + stale permanent delete. The shared theme is data preservation first. When the provider cannot prove a mutation is safe, it should preserve both versions, block and refresh, or fail @@ -299,55 +310,57 @@ closed rather than silently overwrite remote or local work. Lower risk today: -- Stale content replace, because the provider creates a renamed conflict copy. -- Stale rename, move, trash, and delete, because the provider blocks the server - mutation before overwriting newer remote state. +- Stale and raced content replacement, because the provider uses ETag/If-Match + and creates a renamed conflict copy. +- Rename, move, and trash conflicts, because local intent is applied to stable + IDs and collisions preserve both. +- Stale permanent delete, because it is blocked and returned as + `.deletionRejected`. - Cursor races, because guarded snapshot saves prevent stale cache writers from regressing stored cursor state. - Malformed listing pages, because cursor/action anomalies fail closed. Medium risk today: -- File create-create, because files use `conflict=version` and depend on kDrive - version retention. -- Rename-to-existing-name, because sibling collision behavior is still delegated - to kDrive unless the stale metadata check catches a concurrent change. -- Operation timeout after server success, because there is no idempotency or - pending-operation journal. +- Provider restart after repeated upload failure, because bytes survive but + retry scheduling still belongs to File Provider. +- Directory creates, because the API offers no provider idempotency token. +- Case-only collisions, because there is no case-folded sibling policy. Higher risk today: -- Folder create conflicts, because no explicit conflict policy is passed. +- Permanent delete still has a fetch-then-delete race because Infomaniak does + not document conditional ETag deletion. +- Failed preflight requests retain staged bytes but cannot always index a + recovery event before remote metadata (especially the parent) is known. +- Unsupported File Provider metadata is returned as still pending but has no + kDrive/local metadata implementation, which can produce a soft lock. - Parent deleted while child is created or modified, because there is no - recovered folder policy. -- Failed conflict upload recovery, because staged bytes are retained but not yet - surfaced through an automatic retry UI. -- Case-only name conflicts, because there is no case-folded sibling policy. + recovered-folder policy. ## Recommended Safe Direction For a data-loss-averse provider, future work should still: -1. Add a SQLite pending-operation journal for creates, modifies, deletes, and - retries. -2. Store local upload bytes durably before every network send, not only stale - content conflict copies. -3. Prefer `conflict=rename` for file creates when preserve-both is desired. -4. Pass an explicit directory conflict policy when creating folders. -5. Return `NSFileProviderError.filenameCollision` for local name collisions that - File Provider can safely rename. -6. Move from timestamp-derived versions to authoritative kDrive revision tokens - if available. -7. Treat parent-deleted scenarios as recovery cases with staged child contents. -8. Add case-normalized collision checks for create, rename, and move. -9. Consider bounce-rename handling for rename swaps. -10. Reconcile by stable item ID where possible, and by parent plus filename only - before the server assigns an ID. +1. Add a provider-owned pending-operation scheduler with exponential backoff, + indefinite retry, a 24-hour Needs Attention transition, and Retry Now. +2. Add type-aware and case-normalized collision preflight. +3. Treat parent-deleted scenarios as recovered-folder cases with staged child + contents. +4. Add conditional/revision parity for large upload sessions. +5. Seek or design a conditional permanent-delete contract with Infomaniak. +6. Add bounce-rename handling for rename swaps. +7. Implement or explicitly reject every unsupported File Provider metadata + field so it cannot remain pending forever. ## Bottom Line -The three high-risk guardrails are now addressed for snapshot races, malformed -listing state, and stale existing-item mutations. The provider is still not a -full conflict-safe sync engine until pending operations, broader staged uploads, -explicit create collision handling, stronger server version tokens, and local -name-collision policy are in place. +Guardrails are now in place for strong content versions, conditional content +replacement, collision-safe creates, combined fields, fail-on-conflict, +deletion rejection, staged recovery copies, error-resolution signalling, +snapshot races, and malformed listings. The provider is still not a complete +conflict-safe sync engine until provider-owned retry scheduling, recovered +folders, unsupported-metadata handling, large-upload conditional parity, and a +conditional permanent-delete primitive are addressed. The maintained finding +list is in +[Conflict Resolution Truth Table And Safety Register](CONFLICT_RESOLUTION_TRUTH_TABLE.md). diff --git a/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md b/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md new file mode 100644 index 0000000..9a2fa6f --- /dev/null +++ b/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md @@ -0,0 +1,210 @@ +# Conflict Resolution Truth Table And Safety Register + +> [!IMPORTANT] +> This is a mission-critical data-safety document. It must describe the behavior +> of the current implementation, including known unsafe or incomplete paths. Any +> change that can alter conflict detection, mutation ordering, server conflict +> policy, retry behavior, or user recovery must update this file in the same +> change. + +This document is the normative conflict-resolution truth table and open safety +register for `potassiumProvider`. [Conflict Cases And Resolution](CONFLICTS.md) +provides the broader design narrative; when the two documents disagree, this +audited truth table takes precedence and the inconsistency must be corrected. + +## Audit Status + +- Last source audit: 2026-08-02 +- Audited baseline: `codex/conflict-resolution-hardening` working tree +- Validation: + - `potassiumChannel`: `swift test` — 558 tests passed + - macOS: `KDriveMutationCoordinatorTests` — 25 tests passed (the selected + test plan executes this suite twice) + - macOS app build passed with code signing disabled + - iOS Simulator app build passed on iPhone 17 / iOS 26.5 with code signing + disabled + - generic visionOS app build passed with code signing disabled + - Full macOS unit run: conflict/version/recovery tests passed; three existing + snapshot-store tests were flaky under parallel execution and the full + scheme UI runner could not finish because the host disk was full +- Live kDrive collision validation: not performed; server-dependent behavior is + identified explicitly below +- Finding state vocabulary: **Open**, **Mitigated**, or **Resolved** + +Unit tests validate isolated coordinator operations, including a remote change +between preflight and conditional upload. They do not yet invoke combined File +Provider `changedFields` through an end-to-end extension callback or prove +system retry/backoff behavior across extension restarts. + +## Scope And Evidence + +The table is derived from these implementation boundaries: + +- [`PotassiumFileProviderExtension`](../potassiumProviderFileProvider/PotassiumFileProviderExtension.swift) + dispatches create, modify, trash, and delete callbacks and reports their + completion state to File Provider. +- [`KDriveMutationCoordinator`](../PotassiumProviderCore/KDriveMutationCoordinator.swift) + compares base versions and selects mutation or conflict-copy behavior. +- [`KDriveVersionConflictResolver`](../PotassiumProviderCore/KDriveModels.swift) + defines content and metadata equality. +- [`PotassiumKDriveService`](../PotassiumProviderCore/KDriveRemoteService.swift) + selects kDrive conflict flags and constructs requests. +- [`FileProviderRuntime`](../potassiumProviderFileProvider/FileProviderRuntime.swift) + maps provider and API failures to File Provider errors. +- [`FileProviderEnumerator`](../potassiumProviderFileProvider/FileProviderEnumerator.swift) + validates listing, cursor, and snapshot state. +- [`ProviderEventStore`](../PotassiumProviderCore/ProviderEventStore.swift) and + the Activities UI record conflict state but do not replay failed mutations. + +Apple's replicated File Provider contract is also normative: + +- [`modifyItem`](https://developer.apple.com/documentation/fileprovider/nsfileproviderreplicatedextension/modifyitem(_:baseversion:changedfields:contents:options:request:completionhandler:)) + may contain content, filename, and parent changes together. Filename and + contents must be synchronized together, and unapplied fields must be returned + as still pending. +- [`deletionRejected`](https://developer.apple.com/documentation/fileprovider/nsfileprovidererror/deletionrejected) + lets the system recreate a deletion that the provider rejected. +- [`filenameCollision`](https://developer.apple.com/documentation/fileprovider/nsfileprovidererror/code/filenamecollision) + lets the system resolve a collision and retry. + +Infomaniak's public API contract documents the primitives used here: + +- [`Get File/Directory`](https://developer.infomaniak.com/docs/api/get/3/drive/%7Bdrive_id%7D/files/%7Bfile_id%7D) + and [`Get files in directory`](https://developer.infomaniak.com/docs/api/get/3/drive/%7Bdrive_id%7D/files/%7Bfile_id%7D/files) + list `etag` as an opt-in `with` resource and return `revised_at`. +- [`Upload`](https://developer.infomaniak.com/docs/api/post/3/drive/%7Bdrive_id%7D/upload) + documents `If-Match` as the ETag of a specific file version, accepts stable + `file_id`, `client_token`, and `total_chunk_hash`, and can return `etag`. +- Infomaniak does not specify one exclusive stale-`If-Match` response status; + the provider treats both HTTP 409 and 412 as conditional conflicts. +- No conditional ETag parameter is documented for rename, move, trash, or + permanent delete. That absence is why permanent-delete race risk remains + explicitly open. + +## Predicate Legend + +| Symbol | Meaning | +| --- | --- | +| `C` | The versioned File Provider base contains the same stable item ID and authoritative ETag as the latest remote item. | +| `B` | The base item ID, name, and parent equal the latest remote state. `updatedAt`-only drift is ignored. | +| `D` | The latest remote name and parent already equal the requested final state. | +| `U` | The selected remote mutation or upload succeeds. | + +`contentVersion` is versioned JSON. Item ID plus ETag are authoritative; +`revisedAt` and size are diagnostic. Legacy timestamp versions and missing +ETags fail closed into preserve-both or `.failOnConflict` behavior. + +## Core Mutation Truth Table + +| Request or conflict | Predicate | Current action | Server mutation | Data-loss assessment | User recovery | +| --- | --- | --- | --- | --- | --- | +| New file with no collision known locally | Always | Stage first, then upload by parent/name with `conflict=rename`, SHA-256, and deterministic `client_token`; request `with=etag`. Remove the stage only after success. | Creates an item | Low. The returned server item is authoritative and replay uses the same token. | None if successful; a failed create retains an unindexed staged copy. | +| New file collides with an existing name or type | Server applies rename policy | Create a visible uniquely named item; never request server-side overwrite/versioning. | Creates a second item | Low byte-loss risk; a safe duplicate is possible for `.mayAlreadyExist`. | Compare/delete the duplicate if it represents the same file. | +| New directory collides by name or type | Recognized HTTP 409, or named 422 collision | Retry once with a conflict filename. | Creates a second directory | Low byte-loss risk, but response-shape coverage is not live-validated. | Rename/merge folders if the response was not recognized. | +| Local content edit; remote unchanged | `C`, conditional upload succeeds | Stage first, then replace by `file_id` with `If-Match`, SHA-256, and deterministic token; remove stage only after success. | Conditional content replace | Low. A remote race cannot silently pass the checked ETag. | None. | +| Remote changes after preflight | `C`, conditional upload rejects with 409/412 | Refetch and upload a renamed conflict copy from the same staged bytes. | Creates a second item | Low. Both versions are preserved. | Compare or merge the visible files. | +| Local content edit vs already-changed remote content | `!C && U` | Upload a renamed conflict copy and leave the original unchanged. | Creates a second item | Low. Both versions are preserved. | Compare or merge the visible files. | +| `.failOnConflict` content conflict | `!C`, or conditional 409/412 | Do not mutate kDrive; return `.localVersionConflictingWithServer`; keep staged bytes and record recovery path. | No | Low immediate loss risk. This is intentional user-intervention behavior. | Reveal/export recovery copy, compare versions, then retry the desired change. | +| Staging fails | Stage write fails before any server mutation | Propagate local storage failure. | No | High: provider could not obtain its own durable copy, though File Provider still owns the callback URL. | Free local space and retry; no provider copy exists. | +| Preflight lookup fails after staging | `item(...)` fails | Return mapped retryable error and retain deterministic staged bytes. | No | Medium: bytes survive, but an event cannot always be indexed without authoritative parent metadata. | Let File Provider retry; unindexed copies require support/developer recovery. | +| Replace/conflict upload fails after staging | `!U` | Return mapped retryable error; retain stage; indexed conflict failures appear in Activities. | No confirmed success | Low immediate loss risk; provider-owned scheduling is still absent. | File Provider retries; Activities can reveal/export indexed recovery bytes. | +| Rename vs remote rename/move | Stable file ID exists | Local name intent wins. Retry a recognized collision with a unique conflict name, then refetch. | Renames item | Low byte-loss risk. The remote same-field name loses by explicit policy. | Inspect final unique name; no byte merge required. | +| Retried rename already reflected remotely | `D` | Return latest item without another mutation. | No | Safe idempotent success. | None. | +| Move-only vs remote rename | Destination differs; local name unchanged | Move stable ID with `name=nil`, preserving the remote rename; kDrive uses `conflict=rename`. | Moves item | Low. Independent fields merge automatically. | None. | +| Combined move+rename vs remote metadata | Stable file ID exists | Local destination and name win; move uses `conflict=rename`; refetch authoritative item. | Moves/renames item | Low byte-loss risk. Same-field metadata follows explicit local intent. | Inspect server-selected unique name if collision occurs. | +| Trash vs remote content/metadata | Stable file ID exists | Apply local trash intent after other requested fields. Remote bytes remain recoverable in trash. | Trashes item | Low immediate risk; trash is reversible. | Restore from trash if the intent was wrong. | +| Permanent delete; remote matches base | `C && B` | Delete trashed item by stable ID. | Destructive delete | Residual high-impact race: Infomaniak documents no conditional delete token. | None after accepted deletion. | +| Permanent delete vs remote change | `!(C && B)` | Do not delete; return `.deletionRejected` containing latest trashed item. | No | Low. File Provider can recreate the item locally. | Review the recreated item and retry deletion if still desired. | +| Permanent delete already completed | Latest lookup returns 404 | Return idempotent success. | No | Safe; prevents ghost/stuck deletion. | None. | + +## Combined `changedFields` Truth Table + +File Provider can send multiple changes in one `modifyItem` callback. The +extension applies all supported fields in this order: + +1. parent move plus optional filename, or filename-only rename +2. contents plus its modification date +3. standalone modification date +4. move to trash + +Applied fields are removed from `stillPendingFields`. Unsupported fields remain +pending and are never falsely acknowledged. + +| Fields in one callback | Branch executed | Applied remotely | Silently unhandled | Assessment | +| --- | --- | --- | --- | --- | +| Contents + filename | Rename, then contents | Both; content replaces the same stable file ID under the requested name | None | Automatic. Conditional content race still preserves both. | +| Contents + parent | Move, then contents | Both; move-only preserves an independent remote rename | None | Automatic. | +| Contents + filename + parent | Combined move/rename, then contents | All three | None | Automatic. | +| Contents + move to trash | Contents first, trash last | New contents are conditionally replaced or preserved as a conflict item; affected item(s) are then trashed | None | No silent byte loss; conflict copies remain recoverable in trash. | +| Filename + parent | Combined move/rename | Both | None | Automatic with `conflict=rename`. | +| Content modification date only | Date update | `last_modified_at` | None | Automatic; returned item is refetched with ETag. | +| Unsupported metadata fields only | Refetch | Nothing | All unsupported fields returned pending | No false success, but repeated pending fields can soft-lock until support is implemented. | + +## Open Safety Findings + +| ID | Severity | Finding | Consequence | State | +| --- | --- | --- | --- | --- | +| `CR-001` | Critical | Combined `changedFields` were mutually exclusive and falsely reported complete. | The implementation now applies move/rename, content/date, and trash in order and returns unsupported fields pending. End-to-end extension callback coverage is still required. | **Mitigated** | +| `CR-002` | High | Existing-item mutations had a fetch-then-mutate race. | Content now uses ETag/`If-Match`; permanent delete still lacks a documented conditional server primitive. | **Mitigated** | +| `CR-003` | High | Content replacement addressed latest parent/name instead of stable ID. | Replacement now uses `file_id`, authoritative ETag, and `If-Match`; conditional races preserve both. | **Resolved** | +| `CR-004` | High | Content versions used only `modifiedAt`. | Versions now contain stable item ID plus ETag; legacy/missing ETags fail closed. | **Resolved** | +| `CR-005` | High | Latest lookup happened before staging. | Bytes now stage first, but a preflight failure may leave an unindexed recovery copy because parent metadata is unavailable. | **Mitigated** | +| `CR-006` | High | Contents+trash ignored the new bytes. | Content is replaced/preserved before trash; conflict item and original are both trashed when required. | **Resolved** | +| `CR-007` | Medium | Failed uploads stranded private staged bytes. | Indexed failures have Activities reveal/export and deterministic replay; provider-owned scheduling and Retry Now remain absent. | **Mitigated** | +| `CR-008` | Medium | Stale delete/collision errors caused avoidable soft locks. | Stale permanent delete returns `.deletionRejected`; recognized collisions auto-rename. No `filenameCollision` bounce is needed for handled cases. | **Resolved** | +| `CR-009` | Medium | Mutation replay was not idempotent. | Direct file create/replace/conflict copy use deterministic client tokens and hashes; directory create and `.mayAlreadyExist` identity reconciliation remain gaps. | **Mitigated** | +| `CR-010` | Medium | Recoverable errors had no resolution signal. | Successful metadata/content/mutation operations signal authentication, quota, reachability, and synchronization errors resolved. | **Resolved** | +| `CR-011` | Low | Listing cursor, action, and snapshot anomalies fail closed. | Folder availability can be temporarily blocked, but ambiguous snapshots are not committed and remote data is not mutated. | **Mitigated** | +| `CR-012` | Low | Stale content edits use staged renamed preserve-both. | Both byte streams are preserved, including a 409/412 race after preflight. | **Resolved** | +| `CR-013` | High | Infomaniak documents no conditional ETag for permanent delete. | A remote change between delete preflight and accepted deletion could be irrecoverable. | **Open** | +| `CR-014` | Medium | Unsupported File Provider metadata remains pending without an implementation. | The change is not lost, but File Provider can repeatedly resubmit it and soft-lock the item. | **Open** | +| `CR-015` | Medium | Retry cadence is delegated to File Provider; there is no provider-owned indefinite scheduler or Retry Now action. | Staged bytes survive, but recovery can depend on system resubmission or manual export. | **Open** | +| `CR-016` | Medium | New-file create bytes were not provider-staged before the initial upload. | Creates now stage deterministically before request construction and remove the copy only after confirmed success; regression coverage verifies failed creates retain bytes. | **Resolved** | + +## User-Recovery Matrix + +| State | Automatic recovery | User can fix in current app | External/manual recovery | +| --- | --- | --- | --- | +| Successful renamed conflict copy | Both versions are created automatically; no semantic merge is attempted. | Open both visible files and decide which content to keep. | Finder/Files or kDrive clients can merge/delete copies. | +| `.failOnConflict` | Mutation is deliberately stopped and staged bytes retained. | Activities reveals/exports indexed recovery bytes; user chooses the winning/merged content and retries. | Finder/Files or another kDrive client can resolve the versions. | +| Stale rename or move | Local same-field intent applies automatically; move-only preserves remote rename. | Usually none; inspect a collision-selected unique name if desired. | Resolve unusual case-only/type collisions in another client. | +| Trash after remote change | Trash intent applies and remains reversible. | Restore from trash if the local intent was wrong. | Restore through any kDrive client. | +| Stale permanent delete | `.deletionRejected` asks File Provider to recreate the latest item. | Review the recreated item and retry deletion. | Another kDrive client can inspect/delete it. | +| Failed indexed content upload | File Provider receives a retryable error and can resubmit with the same token/hash. | Activities reveals/exports the staged copy; after 24 hours it is labelled Needs Attention. | Support can recover an unindexed stage from app-group `ConflictStaging`. | +| Failed new-file create | File Provider can resubmit with the same deterministic token/hash and provider-owned bytes remain staged. | No indexed Activities action yet. | Support can recover the stage from app-group `ConflictStaging`. | +| Name collision rejected in an unrecognized shape | Recognized 409/422 collisions auto-rename. | Choose a unique name and retry. | Resolve collision through another kDrive client. | +| Invalid listing or cursor state | Anchor reset/rebuild occurs for supported cases. | Usually no direct fix beyond retrying later. | Server/provider correction may be required for repeatedly invalid responses. | +| Missing authentication | Token refresh is attempted when possible. | Yes: sign in again. | Account or credential repair may be required. | +| Quota exhausted | No upload until quota is available. | Yes, outside the provider: free space or increase quota. | kDrive account management. | +| Unsupported metadata remains pending | File Provider can resubmit, but the provider has no handler. | No reliable current fix; undo the originating metadata action if possible. | Requires a provider update; this is a soft-lock class, not byte loss. | + +## Required Maintenance Procedure + +This file must be reviewed and updated in the same change whenever any of the +following changes: + +- `createItem`, `modifyItem`, or `deleteItem` dispatch and completion handling; +- `changedFields`, `stillPendingFields`, or File Provider callback options; +- content or metadata version construction and comparison; +- create, replace, rename, move, trash, or delete request construction; +- kDrive conflict flags, stable identifiers, ETags, revisions, checksums, or + idempotency tokens; +- error classification or File Provider error mapping; +- conflict staging, persistence, retry, cleanup, or user-recovery UI; +- enumeration validation, sync anchors, snapshot races, or reconciliation; +- tests that prove or invalidate any row or finding. + +Every applicable change must: + +1. Update the affected truth-table rows and finding states. +2. Add or update regression tests for every changed decision cell. +3. Record the new audit date, validation command, and server-validation status. +4. Re-check Apple's current replicated File Provider documentation. +5. Keep uncertain kDrive behavior marked server-dependent until a guarded live + test or authoritative API contract proves it. +6. Treat an inaccurate or stale table as a release-blocking data-safety defect. + +A finding may move to **Resolved** only when the implementation, tests, user +recovery behavior, and this document agree. Do not close a finding solely +because an error is logged, bytes happen to remain in a private directory, or a +server currently appears to preserve an older version. diff --git a/doc/FILE_LISTING_CACHING_IMPROVEMENTS.md b/doc/FILE_LISTING_CACHING_IMPROVEMENTS.md index 80694c8..0341ba1 100644 --- a/doc/FILE_LISTING_CACHING_IMPROVEMENTS.md +++ b/doc/FILE_LISTING_CACHING_IMPROVEMENTS.md @@ -46,16 +46,20 @@ Implemented: mutations - preserve-both stale content handling through app-group staging plus renamed `conflict=rename` upload -- stale rename, move, trash, and delete blocked before server mutation +- ETag/`If-Match` conditional replacement by stable `file_id`, with a + post-preflight 409/412 converted to a preserve-both conflict copy +- local-intent rename and move handling, reversible local-intent trash, and + stale permanent delete rejected before server mutation - explicit `KDriveUploadConflictStrategy` for file uploads Partially addressed: -- conflict bytes are staged for stale content conflict copies only; there is not - yet a durable pending-operation table or automatic staged-byte retry workflow -- stale destructive/metadata mutations return a platform-compatible - `.cannotSynchronize` error with recovery text on this target, rather than a - more specific version-unavailable error +- existing-item bytes are staged before network preflight and retained after + failure, with reveal/export recovery for indexed events; there is not yet a + provider-owned retry scheduler or complete staged-operation journal +- stale permanent delete returns File Provider's `.deletionRejected`; the API + still exposes no documented conditional ETag primitive for closing the race + between delete preflight and accepted deletion - snapshot saves now publish immutable generations atomically and retain the active generation plus two predecessors for stable item/change paging diff --git a/doc/LISTING_AND_VERSIONING.md b/doc/LISTING_AND_VERSIONING.md index 0238596..0a1b6d9 100644 --- a/doc/LISTING_AND_VERSIONING.md +++ b/doc/LISTING_AND_VERSIONING.md @@ -215,14 +215,18 @@ stale writer does not overwrite newer cache state. `FileProviderItem` maps `KDriveRemoteItem` into `NSFileProviderItemVersion`: -- `contentVersion`: `modifiedAt.timeIntervalSince1970` -- `metadataVersion`: `id`, `updatedAt`, `name`, and `parentID` +- `contentVersion`: versioned JSON containing stable item ID, ETag, + `revisedAt`, and size; item ID plus ETag are authoritative +- `metadataVersion`: versioned JSON containing ID, `updatedAt`, name, and + parent ID -This is a lightweight versioning scheme. The extension compares the File -Provider `baseVersion` with freshly fetched kDrive metadata before content, -metadata, trash, and delete mutations. +Legacy timestamp content versions and missing ETags fail closed. The extension +compares the File Provider `baseVersion` with freshly fetched kDrive metadata +before mutations; matching content replacement also sends the ETag through +`If-Match` so a remote edit after preflight cannot be overwritten silently. Stale content replacement preserves both by uploading the local bytes as a -renamed conflict copy. Stale rename, move, trash, and permanent delete requests -are blocked before the server is mutated. See [Conflicts](CONFLICTS.md) for the -remaining limitations. +renamed conflict copy. Rename and move apply local same-field intent while +preserving independent metadata fields, trash applies last and is reversible, +and stale permanent delete is rejected before mutation. See +[Conflicts](CONFLICTS.md) for the remaining limitations. diff --git a/doc/MUTATIONS.md b/doc/MUTATIONS.md index 23871b6..5b21f32 100644 --- a/doc/MUTATIONS.md +++ b/doc/MUTATIONS.md @@ -6,9 +6,11 @@ runtime state, converts File Provider inputs, records activity/conflict audit events, and delegates conflict-sensitive kDrive decisions to `KDriveMutationCoordinator` in `PotassiumProviderCore`. -The current implementation does not maintain a durable pending-operation queue. -It also does not update SQLite snapshots directly after mutations. Instead, -later listing or change enumeration reconciles snapshots from kDrive state. +The current implementation does not maintain its own pending-operation +scheduler; retry cadence remains delegated to File Provider. File content bytes +are staged durably before create upload or existing-item mutation preflight. +SQLite snapshots are not updated +directly after mutations; later enumeration reconciles kDrive state. ## Create @@ -24,8 +26,14 @@ For files: - Resolve the parent File Provider identifier to a kDrive parent ID. - Read bytes from the local contents URL supplied by File Provider. +- Stage the bytes deterministically in app-group `ConflictStaging` before the + first network send. - Call `uploadFile(driveID:parentID:fileName:contents:lastModifiedAt:conflictStrategy:)`. -- Upload uses `conflict: "version"`. +- Upload uses `conflict: "rename"`, a SHA-256 `total_chunk_hash`, and a + deterministic `client_token`, so a collision preserves both visible files + and replay is idempotent at the API boundary. +- Remove the staged copy only after the server confirms success; retain it if + request construction or upload fails. - Return the created server item as `FileProviderItem`. SQLite snapshots are not directly edited after create. The created item appears @@ -35,42 +43,60 @@ in snapshots when enumeration or advanced listing changes see it. When `modifyItem(...)` includes `.contents`, `KDriveMutationCoordinator`: -- Fetches the latest kDrive item metadata. +- Stages the local bytes in app-group `ConflictStaging` before any network + preflight. +- Computes a SHA-256 `total_chunk_hash` and deterministic `client_token`. +- Fetches the latest kDrive metadata with `with=etag`. - Compares the File Provider content `baseVersion` with the latest remote - content version. + content version by stable item ID and authoritative ETag. Legacy timestamp + versions and missing ETags fail closed into preserve-both handling. - If the versions match, calls - `replaceFile(driveID:fileID:contents:lastModifiedAt:)`. -- Replace uses kDrive upload with `fileId` and `conflict: "version"`. + `replaceFile(driveID:fileID:expectedETag:clientToken:contentHash:...)`. +- Replacement uses kDrive upload with `file_id` and `If-Match`, not a + potentially stale parent/name pair. - The server-returned item is returned to File Provider. -- If the remote content changed, stages the local bytes in the app group and - uploads them as a renamed conflict copy with `conflict: "rename"`. +- If remote content changed, or a 409/412 conditional race is lost, uploads the + staged bytes as a renamed conflict copy with `conflict: "rename"`. The original remote item is left untouched and the conflict item is returned. +- If File Provider requests `.failOnConflict`, no conflict copy is uploaded; + `.localVersionConflictingWithServer` is returned and staged bytes remain for + user recovery. +- Successful replacement or conflict-copy upload removes the staged bytes. +- Failed uploads retain a deterministic staged copy. File Provider can retry + the callback, and indexed copies can be revealed/exported from Activities. -The provider does not yet maintain a durable pending-operation table for the -staged conflict upload. See [Persistence](PERSISTENCE.md). +`modifyItem(...)` applies combined fields in this order: move/rename, contents, +standalone modification date, then trash. It returns every unapplied field in +`stillPendingFields` instead of acknowledging it as completed. + +The provider does not yet maintain a provider-owned retry schedule. See +[Persistence](PERSISTENCE.md). + +The exact decision matrix, combined-field behavior, and open data-safety +findings are maintained in +[Conflict Resolution Truth Table And Safety Register](CONFLICT_RESOLUTION_TRUTH_TABLE.md). ## Base-Version Checks `KDriveVersionConflictResolver` compares the incoming `NSFileProviderItemVersion` with freshly fetched `KDriveRemoteItem` versions: -- content replacement checks `contentVersion` -- rename and move check `metadataVersion` -- trash and permanent delete check both content and metadata versions +- content replacement checks the authoritative `(itemID, ETag)` tuple +- rename, move, and trash apply local intent to the latest stable item ID +- permanent delete checks both authoritative content and metadata versions -Stale metadata, trash, and delete mutations throw -`KDriveMutationConflictError.staleVersion` before sending a server mutation. The -File Provider adapter maps that to `.cannotSynchronize` with a recovery -suggestion to refresh and retry. +Stale permanent deletes throw `KDriveMutationConflictError.staleVersion` before +the server mutation. The adapter returns `.deletionRejected` with the latest +trashed item so File Provider can recreate it. Already-missing deletes succeed +idempotently. ## Rename When `modifyItem(...)` includes `.filename` and not a parent change: -- The extension fetches fresh item metadata. -- It compares the File Provider metadata `baseVersion` with the latest remote - metadata version. -- If the versions match, it calls `renameItem(driveID:fileID:name:)`. +- The extension fetches fresh item metadata and applies the local name to the + stable item ID (local same-field intent wins). +- A recognized 409/422 collision retries with a deterministic conflict name. - It then fetches fresh item metadata with `item(...)`. - The fetched item is returned to File Provider. @@ -80,13 +106,11 @@ No local sibling-name preflight is currently performed. When `modifyItem(...)` includes `.parentItemIdentifier`: -- The extension fetches fresh item metadata. -- It compares the File Provider metadata `baseVersion` with the latest remote - metadata version. -- If the versions match, it resolves the destination parent ID. +- The extension fetches fresh metadata and resolves the destination parent ID. - It calls `moveItem(driveID:fileID:destinationParentID:name:)`. - Move uses `conflict: "rename"`. -- If the filename also changed, the new name is sent with the move. +- If the filename also changed, the local name is sent with the move. For a + move-only request, an independent remote rename is preserved. - The extension fetches fresh item metadata and returns it. Move still has the most preserve-both-friendly server conflict flag because it @@ -97,9 +121,10 @@ asks kDrive to rename on collision. When `modifyItem(...)` changes the parent to `.trashContainer`: - The extension fetches fresh item metadata. -- It compares both content and metadata base versions with the latest remote - versions. -- If the versions match, it calls `trashItem(driveID:fileID:)`. +- It applies the local trash intent to the stable item ID. Concurrent remote + edits are preserved in trash and remain restorable. +- For combined content+trash, content is replaced or preserved as a conflict + copy before both affected items are moved to trash. - It completes without returning an updated item. Later enumeration reconciles the item removal from its old container and its @@ -114,12 +139,16 @@ appearance in trash. - Compare both content and metadata base versions with the latest remote versions. - If the versions match, call `deleteTrashedItem(driveID:fileID:)`. -- Return success or a mapped error. +- If the item changed, return File Provider's `.deletionRejected` error with the + latest trashed item so the system can restore local consistency. +- If the item is already absent, return idempotent success. This does not delete regular non-trash items directly. Moving to trash is handled through `modifyItem(...)`. -Stale deletes are blocked before server mutation. +Stale deletes are blocked before server mutation. Infomaniak does not document +a conditional ETag parameter for the permanent-delete endpoint, so a final +fetch-to-delete race remains a known limitation. ## Server-Authoritative Return Flow @@ -131,9 +160,10 @@ The mutation callbacks still use server state for returned metadata: - Rename and move fetch the item again after the server operation. - Trash and delete return success without directly editing snapshots. -This keeps the local provider from inventing metadata, while the base-version -preflight prevents stale destructive or metadata mutations from overwriting newer -remote state. See [Conflicts](CONFLICTS.md). +This keeps the local provider from inventing metadata. Conditional ETag upload +closes the content replacement race; metadata applies the selected local-intent +policy; permanent deletion retains a documented residual race. See +[Conflicts](CONFLICTS.md). ## Reconciliation After Mutation diff --git a/potassiumProvider.xcodeproj/project.pbxproj b/potassiumProvider.xcodeproj/project.pbxproj index 1bf7d17..7dca86a 100644 --- a/potassiumProvider.xcodeproj/project.pbxproj +++ b/potassiumProvider.xcodeproj/project.pbxproj @@ -1255,8 +1255,8 @@ isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/OpenCow42/potassiumChannel.git"; requirement = { - kind = upToNextMajorVersion; - minimumVersion = 0.2.0; + branch = "codex/kdrive-etag-revisions"; + kind = branch; }; }; C0DCD8ED2FFA44AB00520215 /* XCRemoteSwiftPackageReference "swift-concurrency" */ = { diff --git a/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 34cbefa..9ed49fb 100644 --- a/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,13 +1,13 @@ { - "originHash" : "613cbb0d4ce50189d97b72e2dc228bda845d46b491781fb0143dbeb79a057a19", + "originHash" : "4ca0a296a28bfdc2857c81a1f70f11cc9891a6c43de2f84a68c780c40bcf7fe3", "pins" : [ { "identity" : "potassiumchannel", "kind" : "remoteSourceControl", "location" : "https://github.com/OpenCow42/potassiumChannel.git", "state" : { - "revision" : "8a6d236d69c381c17f334b66dd4075ef2e0b7d89", - "version" : "0.2.0" + "branch" : "codex/kdrive-etag-revisions", + "revision" : "1bed1613782e3d0a6f707e3ddc6ebda5e0bddec5" } }, { diff --git a/potassiumProvider/ProviderActivityTimelineRow.swift b/potassiumProvider/ProviderActivityTimelineRow.swift index acf07c3..5b2deb8 100644 --- a/potassiumProvider/ProviderActivityTimelineRow.swift +++ b/potassiumProvider/ProviderActivityTimelineRow.swift @@ -192,6 +192,14 @@ private struct ConflictActivityDetails: View { if let stagedUploadRelativePath = event.stagedUploadRelativePath { LabeledContent("Staged upload", value: stagedUploadRelativePath) + if Date().timeIntervalSince(event.detectedAt) >= 24 * 60 * 60, + event.resolutionState != .automaticallyResolved { + Label("Needs Attention", systemImage: "exclamationmark.triangle.fill") + .foregroundStyle(.orange) + } + if let stagedUploadURL { + StagedUploadRecoveryAction(url: stagedUploadURL) + } } ProviderItemAction( @@ -236,6 +244,39 @@ private struct ConflictActivityDetails: View { } return lines.joined(separator: "\n") } + + private var stagedUploadURL: URL? { + guard let relativePath = event.stagedUploadRelativePath, + relativePath.hasPrefix("ConflictStaging/"), + relativePath.contains("..") == false, + let containerURL = FileManager.default.containerURL( + forSecurityApplicationGroupIdentifier: ProviderConstants.appGroupIdentifier + ) else { + return nil + } + let url = containerURL.appendingPathComponent(relativePath) + return FileManager.default.fileExists(atPath: url.path) ? url : nil + } +} + +private struct StagedUploadRecoveryAction: View { + let url: URL + + var body: some View { + #if os(macOS) + Button { + NSWorkspace.shared.activateFileViewerSelecting([url]) + } label: { + Label("Reveal Recovery Copy", systemImage: "folder") + } + .accessibilityIdentifier("activity.revealStagedUpload") + #else + ShareLink(item: url) { + Label("Export Recovery Copy", systemImage: "square.and.arrow.up") + } + .accessibilityIdentifier("activity.exportStagedUpload") + #endif + } } private struct ProviderActivityDetails: View { diff --git a/potassiumProviderFileProvider/FileProviderRuntime.swift b/potassiumProviderFileProvider/FileProviderRuntime.swift index 064f29a..9425384 100644 --- a/potassiumProviderFileProvider/FileProviderRuntime.swift +++ b/potassiumProviderFileProvider/FileProviderRuntime.swift @@ -176,6 +176,30 @@ struct ProviderErrorMapping { let diagnostic: KDriveProviderActivityErrorDiagnostic } +func signalRecoverableProviderErrorsResolved(for domain: NSFileProviderDomain) async { + guard let manager = NSFileProviderManager(for: domain) else { return } + let errorCodes: [NSFileProviderError.Code] = [ + .notAuthenticated, + .insufficientQuota, + .serverUnreachable, + .cannotSynchronize, + ] + + for code in errorCodes { + let error = NSFileProviderError(code) as NSError + await withCheckedContinuation { continuation in + manager.signalErrorResolved(error) { signalError in + if let signalError { + FileProviderLog.runtime.debug( + "could not signal resolved provider error code(\(error.code, privacy: .public)): \(signalError.localizedDescription, privacy: .public)" + ) + } + continuation.resume() + } + } + } +} + func providerErrorMapping(_ error: Error) -> ProviderErrorMapping { if let fileProviderError = error as? NSFileProviderError { let nsError = fileProviderError as NSError @@ -218,6 +242,17 @@ func providerErrorMapping(_ error: Error) -> ProviderErrorMapping { mappedError: mappedError ) ) + case .localContentConflict: + let mappedError = NSFileProviderError(.localVersionConflictingWithServer) + FileProviderLog.runtime.error("map fail-on-conflict upload to localVersionConflictingWithServer") + return ProviderErrorMapping( + mappedError: mappedError, + diagnostic: providerDiagnostic( + category: .mutationConflict, + originalError: error, + mappedError: mappedError + ) + ) } } diff --git a/potassiumProviderFileProvider/PotassiumFileProviderExtension.swift b/potassiumProviderFileProvider/PotassiumFileProviderExtension.swift index 45f7278..65c9278 100644 --- a/potassiumProviderFileProvider/PotassiumFileProviderExtension.swift +++ b/potassiumProviderFileProvider/PotassiumFileProviderExtension.swift @@ -110,6 +110,7 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli let item = try await loadedRuntime.remote.item(driveID: loadedRuntime.configuration.driveID, fileID: fileID) FileProviderLog.replicatedExtension.debug("resolved item identifier(\(identifier.rawValue, privacy: .public)) kDriveFileID(\(fileID, privacy: .public)) type(\(item.type ?? "unknown", privacy: .public))") + await signalRecoverableProviderErrorsResolved(for: self.domain) await lifecycle.finish(markProgressComplete: true) { completionHandler(FileProviderItem(remoteItem: item, rootFileID: loadedRuntime.configuration.rootFileID), nil) } @@ -206,6 +207,7 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli itemPath: fetchedContents.item.path, summary: "Fetched file contents." ) + await signalRecoverableProviderErrorsResolved(for: domain) let delivered = await lifecycle.finish(markProgressComplete: true) { completionHandler( fetchedContents.temporaryURL, @@ -305,6 +307,7 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli rootFileID: loadedRuntime.configuration.rootFileID ) ) + await signalRecoverableProviderErrorsResolved(for: self.domain) await lifecycle.finish(markProgressComplete: true) { completionHandler(FileProviderItem(remoteItem: createdItem, rootFileID: loadedRuntime.configuration.rootFileID), [], false, nil) } @@ -371,27 +374,115 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli metadataVersion: version.metadataVersion ) var affectedContainerIdentifiers: [NSFileProviderItemIdentifier] = [] + var remainingFields = changedFields + var updatedItem: KDriveRemoteItem? + let requestsTrash = changedFields.contains(.parentItemIdentifier) + && item.parentItemIdentifier == .trashContainer + + // Apply location and name first. kDrive's move endpoint performs + // collision-safe renaming, while rename retries with a unique name. + if changedFields.contains(.parentItemIdentifier), requestsTrash == false { + let parentID = try self.fileID(forParentIdentifier: item.parentItemIdentifier, runtime: loadedRuntime) + updatedItem = try await coordinator.moveItem( + fileID: fileID, + baseMetadataVersion: version.metadataVersion, + destinationParentID: parentID, + name: changedFields.contains(.filename) ? item.filename : nil + ) + remainingFields.remove(.parentItemIdentifier) + remainingFields.remove(.filename) + affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( + forFileIDs: [ + KDriveItemMetadataVersion(data: version.metadataVersion)?.parentID, + parentID, + updatedItem?.parentID, + ], + rootFileID: loadedRuntime.configuration.rootFileID + )) + } else if changedFields.contains(.filename) { + updatedItem = try await coordinator.renameItem( + fileID: fileID, + baseMetadataVersion: version.metadataVersion, + name: item.filename + ) + remainingFields.remove(.filename) + affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( + forFileIDs: [ + KDriveItemMetadataVersion(data: version.metadataVersion)?.parentID, + updatedItem?.parentID, + ], + rootFileID: loadedRuntime.configuration.rootFileID + )) + } - if changedFields.contains(.parentItemIdentifier), item.parentItemIdentifier == .trashContainer { - let latestItem: KDriveRemoteItem + if changedFields.contains(.contents) { + guard let newContents else { + throw NSFileProviderError(.cannotSynchronize) + } do { - latestItem = try await coordinator.trashItem(fileID: fileID, baseVersion: baseVersion) + let result = try await Self.contentTransferLimiter.withPermit { + let data = try Data(contentsOf: newContents, options: .mappedIfSafe) + progress.prepareForByteCount(data.count) + return try await coordinator.replaceContents( + itemIdentifier: item.itemIdentifier.rawValue, + fileID: fileID, + localFilename: item.filename, + baseContentVersion: version.contentVersion, + contents: data, + lastModifiedAt: item.contentModificationDate ?? nil, + failOnConflict: options.contains(.failOnConflict), + transferProgress: progress.attachTransfer + ) + } + updatedItem = result.item + remainingFields.remove(.contents) + remainingFields.remove(.contentModificationDate) + if case .conflictCopy(let conflictItem) = result { + FileProviderLog.replicatedExtension.info("preserved stale content edit as conflict item(\(conflictItem.id, privacy: .public)) original(\(fileID, privacy: .public))") + } + affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( + forFileIDs: [updatedItem?.parentID], + rootFileID: loadedRuntime.configuration.rootFileID + )) } catch let error as KDriveMutationConflictError { await self.recordBlockedConflict( error, - operation: .trash, + operation: .modify, itemIdentifier: item.itemIdentifier.rawValue, itemName: item.filename, runtime: loadedRuntime, - summary: "Trash was blocked because the remote item changed first." + summary: "Upload was blocked because fail-on-conflict was requested." ) throw error } - FileProviderLog.replicatedExtension.info("trash item(\(item.itemIdentifier.rawValue, privacy: .public)) kDriveFileID(\(fileID, privacy: .public))") + } else if changedFields.contains(.contentModificationDate), + let modificationDate = item.contentModificationDate ?? nil { + updatedItem = try await coordinator.updateModificationDate( + fileID: fileID, + date: modificationDate + ) + remainingFields.remove(.contentModificationDate) + } + + // Trash runs last so a combined contents+trash request first + // durably preserves the new bytes. If preservation created a + // conflict copy, both items are moved to trash, not discarded. + if requestsTrash { + let originalItem = try await coordinator.trashItem(fileID: fileID, baseVersion: baseVersion) + if let updatedItem, updatedItem.id != fileID { + _ = try await coordinator.trashItem( + fileID: updatedItem.id, + baseVersion: KDriveItemBaseVersion( + contentVersion: updatedItem.contentVersion, + metadataVersion: updatedItem.metadataVersion + ) + ) + } + remainingFields.remove(.parentItemIdentifier) affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( forFileIDs: [ KDriveItemMetadataVersion(data: version.metadataVersion)?.parentID, - latestItem.parentID + originalItem.parentID, ], rootFileID: loadedRuntime.configuration.rootFileID )) @@ -400,136 +491,59 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli kind: .trash, runtime: loadedRuntime, itemIdentifier: item.itemIdentifier.rawValue, - itemName: latestItem.name, - itemPath: latestItem.path, - summary: "Moved item to trash." + itemName: originalItem.name, + itemPath: originalItem.path, + summary: "Applied pending item changes and moved the item to trash." ) await self.invalidateCachedSnapshotsAndSignal( runtime: loadedRuntime, containerIdentifiers: affectedContainerIdentifiers ) + await signalRecoverableProviderErrorsResolved(for: self.domain) + let completedFields = remainingFields await lifecycle.finish(markProgressComplete: true) { - completionHandler(nil, [], false, nil) + completionHandler(nil, completedFields, false, nil) } return } - let updatedItem: KDriveRemoteItem - if let newContents, changedFields.contains(.contents) { - let result = try await Self.contentTransferLimiter.withPermit { - let data = try Data(contentsOf: newContents, options: .mappedIfSafe) - progress.prepareForByteCount(data.count) - FileProviderLog.replicatedExtension.debug("replace contents for item(\(item.itemIdentifier.rawValue, privacy: .public)) bytes(\(data.count, privacy: .public))") - return try await coordinator.replaceContents( - itemIdentifier: item.itemIdentifier.rawValue, - fileID: fileID, - localFilename: item.filename, - baseContentVersion: version.contentVersion, - contents: data, - lastModifiedAt: item.contentModificationDate ?? nil, - transferProgress: progress.attachTransfer - ) - } - switch result { - case .replaced(let replacedItem): - updatedItem = replacedItem - case .conflictCopy(let conflictItem): - FileProviderLog.replicatedExtension.info("preserved stale content edit as conflict item(\(conflictItem.id, privacy: .public)) original(\(fileID, privacy: .public))") - await self.invalidateCachedSnapshotsAndSignal( - runtime: loadedRuntime, - containerIdentifiers: self.containerIdentifiers( - forFileIDs: [conflictItem.parentID], - rootFileID: loadedRuntime.configuration.rootFileID - ) - ) - await lifecycle.finish(markProgressComplete: true) { - completionHandler(FileProviderItem(remoteItem: conflictItem, rootFileID: loadedRuntime.configuration.rootFileID), [], false, nil) - } - return - } - affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( - forFileIDs: [updatedItem.parentID], - rootFileID: loadedRuntime.configuration.rootFileID - )) - } else if changedFields.contains(.parentItemIdentifier) { - do { - let parentID = try self.fileID(forParentIdentifier: item.parentItemIdentifier, runtime: loadedRuntime) - FileProviderLog.replicatedExtension.debug("move item(\(item.itemIdentifier.rawValue, privacy: .public)) to parentFileID(\(parentID, privacy: .public)) rename(\(changedFields.contains(.filename), privacy: .public))") - updatedItem = try await coordinator.moveItem( - fileID: fileID, - baseMetadataVersion: version.metadataVersion, - destinationParentID: parentID, - name: changedFields.contains(.filename) ? item.filename : nil - ) - affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( - forFileIDs: [ - KDriveItemMetadataVersion(data: version.metadataVersion)?.parentID, - parentID, - updatedItem.parentID - ], - rootFileID: loadedRuntime.configuration.rootFileID - )) - } catch let error as KDriveMutationConflictError { - await self.recordBlockedConflict( - error, - operation: .modify, - itemIdentifier: item.itemIdentifier.rawValue, - itemName: item.filename, - runtime: loadedRuntime, - summary: "Move was blocked because the remote item changed first." - ) - throw error - } - } else if changedFields.contains(.filename) { - do { - FileProviderLog.replicatedExtension.debug("rename item(\(item.itemIdentifier.rawValue, privacy: .public)) filename(\(item.filename, privacy: .private))") - updatedItem = try await coordinator.renameItem( - fileID: fileID, - baseMetadataVersion: version.metadataVersion, - name: item.filename - ) - affectedContainerIdentifiers.append(contentsOf: self.containerIdentifiers( - forFileIDs: [ - KDriveItemMetadataVersion(data: version.metadataVersion)?.parentID, - updatedItem.parentID - ], - rootFileID: loadedRuntime.configuration.rootFileID - )) - } catch let error as KDriveMutationConflictError { - await self.recordBlockedConflict( - error, - operation: .modify, - itemIdentifier: item.itemIdentifier.rawValue, - itemName: item.filename, - runtime: loadedRuntime, - summary: "Rename was blocked because the remote item changed first." - ) - throw error - } + let resolvedItem: KDriveRemoteItem + if let updatedItem { + resolvedItem = updatedItem } else { - updatedItem = try await loadedRuntime.remote.item(driveID: loadedRuntime.configuration.driveID, fileID: fileID) + resolvedItem = try await loadedRuntime.remote.item( + driveID: loadedRuntime.configuration.driveID, + fileID: fileID + ) } - FileProviderLog.replicatedExtension.info("modified item(\(item.itemIdentifier.rawValue, privacy: .public)) kDriveFileID(\(fileID, privacy: .public)) remainingFields([])") + FileProviderLog.replicatedExtension.info("modified item(\(item.itemIdentifier.rawValue, privacy: .public)) kDriveFileID(\(fileID, privacy: .public)) remainingFields(\(String(describing: remainingFields), privacy: .public))") await ProviderEventRecorder.recordActivity( kind: .modify, runtime: loadedRuntime, - itemIdentifier: ProviderEventRecorder.itemIdentifier(for: updatedItem), - itemName: updatedItem.name, - itemPath: updatedItem.path, + itemIdentifier: ProviderEventRecorder.itemIdentifier(for: resolvedItem), + itemName: resolvedItem.name, + itemPath: resolvedItem.path, summary: "Modified item." ) - if updatedItem.isDirectory { + if resolvedItem.isDirectory { affectedContainerIdentifiers.append(NSFileProviderItemIdentifier( - KDriveItemIdentifier.item(updatedItem.id).rawValue + KDriveItemIdentifier.item(resolvedItem.id).rawValue )) } await self.invalidateCachedSnapshotsAndSignal( runtime: loadedRuntime, containerIdentifiers: affectedContainerIdentifiers ) + await signalRecoverableProviderErrorsResolved(for: self.domain) + let completedFields = remainingFields await lifecycle.finish(markProgressComplete: true) { - completionHandler(FileProviderItem(remoteItem: updatedItem, rootFileID: loadedRuntime.configuration.rootFileID), [], false, nil) + completionHandler( + FileProviderItem(remoteItem: resolvedItem, rootFileID: loadedRuntime.configuration.rootFileID), + completedFields, + false, + nil + ) } } catch is CancellationError { await lifecycle.cancel() @@ -582,7 +596,7 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli contentVersion: version.contentVersion, metadataVersion: version.metadataVersion ) - let latestItem: KDriveRemoteItem + let latestItem: KDriveRemoteItem? do { latestItem = try await coordinator.deleteTrashedItem(fileID: fileID, baseVersion: baseVersion) } catch let error as KDriveMutationConflictError { @@ -594,21 +608,35 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli runtime: loadedRuntime, summary: "Delete was blocked because the remote item changed first." ) - throw error + switch error { + case .staleVersion(let remoteItem), .localContentConflict(let remoteItem, _): + throw NSError.fileProviderErrorForRejectedDeletion( + of: FileProviderItem( + remoteItem: remoteItem, + rootFileID: loadedRuntime.configuration.rootFileID, + isTrashed: true + ) + ) + } + } catch where KDriveRemoteErrorClassifier.isNotFound(error) { + // Permanent deletion is idempotent. A missing remote item is + // already the requested final state. + latestItem = nil } FileProviderLog.replicatedExtension.info("deleted trashed item(\(itemIdentifier.rawValue, privacy: .public)) kDriveFileID(\(fileID, privacy: .public))") await ProviderEventRecorder.recordActivity( kind: .delete, runtime: loadedRuntime, itemIdentifier: itemIdentifier.rawValue, - itemName: latestItem.name, - itemPath: latestItem.path, - summary: "Deleted trashed item." + itemName: latestItem?.name, + itemPath: latestItem?.path, + summary: latestItem == nil ? "Trashed item was already deleted." : "Deleted trashed item." ) await self.invalidateCachedSnapshotsAndSignal( runtime: loadedRuntime, containerIdentifiers: [.trashContainer] ) + await signalRecoverableProviderErrorsResolved(for: self.domain) await lifecycle.finish(markProgressComplete: true) { completionHandler(nil) } @@ -928,6 +956,23 @@ public final class PotassiumFileProviderExtension: NSObject, NSFileProviderRepli runtime: runtime, summary: summary ) + case .localContentConflict(let latestItem, let stagedURL): + let now = Date() + await ProviderEventRecorder.saveConflict(KDriveConflictEvent( + detectedAt: now, + resolvedAt: now, + domainIdentifier: runtime.configuration.domainIdentifier, + driveID: runtime.configuration.driveID, + operation: operation, + originalItemIdentifier: itemIdentifier, + originalItemName: itemName ?? latestItem.name, + originalItemPath: latestItem.path, + resolutionState: .blockedRetryable, + automaticallyResolved: false, + resolutionKind: .retainedStagedUploadAfterFailure, + resolutionSummary: "Upload stopped because fail-on-conflict was requested; local bytes were retained for recovery.", + stagedUploadRelativePath: ProviderEventRecorder.relativeStagedPath(for: stagedURL) + ), runtime: runtime) } } diff --git a/potassiumProviderTests/KDriveContextActionTests.swift b/potassiumProviderTests/KDriveContextActionTests.swift index 7651d2d..4a6571f 100644 --- a/potassiumProviderTests/KDriveContextActionTests.swift +++ b/potassiumProviderTests/KDriveContextActionTests.swift @@ -242,19 +242,27 @@ struct KDriveContextActionTests { ) #expect(predicate.contains("fileproviderItems.@count == 1")) } + let providerExtension = try extensionDictionary( + at: repositoryURL.appendingPathComponent("Config/potassiumProviderFileProviderInfo.plist") + ) + #expect(providerExtension["NSExtensionFileProviderSupportsFailingUploadOnConflict"] as? Bool == true) } private func actionDictionaries(at url: URL) throws -> [[String: Any]] { + let extensionDictionary = try extensionDictionary(at: url) + return try #require( + extensionDictionary["NSExtensionFileProviderActions"] as? [[String: Any]] + ) + } + + private func extensionDictionary(at url: URL) throws -> [String: Any] { let data = try Data(contentsOf: url) let propertyList = try PropertyListSerialization.propertyList( from: data, format: nil ) let root = try #require(propertyList as? [String: Any]) - let extensionDictionary = try #require(root["NSExtension"] as? [String: Any]) - return try #require( - extensionDictionary["NSExtensionFileProviderActions"] as? [[String: Any]] - ) + return try #require(root["NSExtension"] as? [String: Any]) } private func actionIdentifier(_ dictionary: [String: Any]) -> String? { diff --git a/potassiumProviderTests/KDriveMachineNamespaceResolverTests.swift b/potassiumProviderTests/KDriveMachineNamespaceResolverTests.swift index fdb462d..e8caf9e 100644 --- a/potassiumProviderTests/KDriveMachineNamespaceResolverTests.swift +++ b/potassiumProviderTests/KDriveMachineNamespaceResolverTests.swift @@ -253,15 +253,19 @@ private actor MachineNamespaceRemote: KDriveFileProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) async throws -> KDriveRemoteItem { throw MachineNamespaceRemoteError.unimplemented } func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem { @@ -281,6 +285,10 @@ private actor MachineNamespaceRemote: KDriveFileProviding { throw MachineNamespaceRemoteError.unimplemented } + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { + throw MachineNamespaceRemoteError.unimplemented + } + func trashItem(driveID: Int, fileID: Int) async throws { throw MachineNamespaceRemoteError.unimplemented } diff --git a/potassiumProviderTests/KDriveMutationCoordinatorTests.swift b/potassiumProviderTests/KDriveMutationCoordinatorTests.swift index 52ff2a6..6004f06 100644 --- a/potassiumProviderTests/KDriveMutationCoordinatorTests.swift +++ b/potassiumProviderTests/KDriveMutationCoordinatorTests.swift @@ -1,13 +1,15 @@ import Foundation import Testing +import PotassiumChannelCore import PotassiumProviderCore @Suite(.serialized) struct KDriveMutationCoordinatorTests { - @Test func fileCreateUploadsWithVersionConflictStrategy() async throws { + @Test func fileCreateUsesCollisionSafeIdempotentUpload() async throws { let createdItem = makeItem(id: 101, name: "New.txt") let remote = RecordingKDriveFileProvider(uploadResult: createdItem) - let coordinator = makeCoordinator(remote: remote) + let stager = RecordingConflictStager(directoryURL: temporaryDirectory()) + let coordinator = makeCoordinator(remote: remote, stager: stager) let contents = Data("new file".utf8) let lastModifiedAt = Date(timeIntervalSince1970: 500) @@ -26,9 +28,37 @@ struct KDriveMutationCoordinatorTests { fileName: "New.txt", contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: .version + conflictStrategy: .rename, + clientToken: KDriveMutationIdentity.clientToken([ + "domain-1", "create", String(Self.parentID), "New.txt", + KDriveMutationIdentity.contentHash(contents), + ]), + contentHash: KDriveMutationIdentity.contentHash(contents) ) ]) + let stagedURL = try #require(await stager.stagedURLs().first) + #expect(await stager.removedURLs() == [stagedURL]) + #expect(!FileManager.default.fileExists(atPath: stagedURL.path)) + } + + @Test func failedFileCreateRetainsStagedBytes() async throws { + let remote = RecordingKDriveFileProvider(uploadError: .uploadFailed) + let stager = RecordingConflictStager(directoryURL: temporaryDirectory()) + let coordinator = makeCoordinator(remote: remote, stager: stager) + let contents = Data("unsent new file".utf8) + + await #expect(throws: RecordingKDriveError.uploadFailed) { + _ = try await coordinator.createFile( + parentID: Self.parentID, + fileName: "New.txt", + contents: contents, + lastModifiedAt: nil + ) + } + + let stagedURL = try #require(await stager.stagedURLs().first) + #expect(try Data(contentsOf: stagedURL) == contents) + #expect(await stager.removedURLs().isEmpty) } @Test func directoryCreateCallsCreateDirectory() async throws { @@ -72,13 +102,19 @@ struct KDriveMutationCoordinatorTests { ) #expect(result == .replaced(replacedItem)) - #expect(await stager.stagedURLs().isEmpty) + let stagedURL = try #require(await stager.stagedURLs().first) + #expect(await stager.removedURLs() == [stagedURL]) #expect(await remote.calls() == [ .item(driveID: Self.driveID, fileID: Self.fileID), .replaceFile( driveID: Self.driveID, - parentID: latestItem.parentID, - fileName: latestItem.name, + fileID: Self.fileID, + expectedETag: try #require(latestItem.etag), + clientToken: KDriveMutationIdentity.clientToken([ + "domain-1", "replace", String(Self.fileID), + try #require(latestItem.etag), KDriveMutationIdentity.contentHash(contents), + ]), + contentHash: KDriveMutationIdentity.contentHash(contents), contents: contents, lastModifiedAt: lastModifiedAt ) @@ -131,7 +167,12 @@ struct KDriveMutationCoordinatorTests { fileName: "Report (conflict - Mac-One.Two - 1970-01-01 00.00.00).pdf", contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: .rename + conflictStrategy: .rename, + clientToken: KDriveMutationIdentity.clientToken([ + "domain-1", "conflict-copy", String(Self.fileID), + latestItem.etag ?? "missing-etag", KDriveMutationIdentity.contentHash(contents), + ]), + contentHash: KDriveMutationIdentity.contentHash(contents) ) ]) @@ -159,6 +200,81 @@ struct KDriveMutationCoordinatorTests { #expect(resolvedConflictItem == conflictItem) } + @Test func failOnConflictRetainsStagedBytesAndDoesNotMutateServer() async throws { + let baseItem = makeItem(id: Self.fileID, name: "Report.txt") + let latestItem = makeItem( + id: Self.fileID, + name: "Report.txt", + modifiedAt: Date(timeIntervalSince1970: 260) + ) + let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem]]) + let stager = RecordingConflictStager(directoryURL: temporaryDirectory()) + let coordinator = makeCoordinator(remote: remote, stager: stager) + let contents = Data("local edit".utf8) + + do { + _ = try await coordinator.replaceContents( + itemIdentifier: String(Self.fileID), + fileID: Self.fileID, + localFilename: "Report.txt", + baseContentVersion: baseItem.contentVersion, + contents: contents, + lastModifiedAt: nil, + failOnConflict: true + ) + Issue.record("Expected fail-on-conflict rejection") + } catch let error as KDriveMutationConflictError { + guard case .localContentConflict(let conflictItem, let stagedURL) = error else { + Issue.record("Expected localContentConflict") + return + } + #expect(conflictItem == latestItem) + #expect(FileManager.default.fileExists(atPath: stagedURL.path)) + } + + #expect(await remote.calls() == [.item(driveID: Self.driveID, fileID: Self.fileID)]) + #expect(await stager.removedURLs().isEmpty) + } + + @Test func conditionalReplaceRaceFallsBackToRenamedConflictCopy() async throws { + let baseItem = makeItem(id: Self.fileID, name: "Report.txt") + let racedItem = makeItem( + id: Self.fileID, + name: "Report.txt", + modifiedAt: Date(timeIntervalSince1970: 260) + ) + let conflictItem = makeItem(id: 303, name: "Report (conflict - Mac-One.Two - 1970-01-01 00.00.00).txt") + let remote = RecordingKDriveFileProvider( + itemResults: [Self.fileID: [baseItem, racedItem]], + uploadResult: conflictItem, + replaceStatusCode: 412 + ) + let stager = RecordingConflictStager(directoryURL: temporaryDirectory()) + let coordinator = makeCoordinator(remote: remote, stager: stager) + let contents = Data("local edit".utf8) + + let result = try await coordinator.replaceContents( + itemIdentifier: String(Self.fileID), + fileID: Self.fileID, + localFilename: "Report.txt", + baseContentVersion: baseItem.contentVersion, + contents: contents, + lastModifiedAt: nil + ) + + #expect(result == .conflictCopy(conflictItem)) + #expect(await remote.calls().map { call in + switch call { + case .item: return "item" + case .replaceFile: return "replace" + case .uploadFile: return "upload" + default: return "other" + } + } == ["item", "replace", "item", "upload"]) + let stagedURL = try #require(await stager.stagedURLs().first) + #expect(await stager.removedURLs() == [stagedURL]) + } + @Test func failedConflictUploadLeavesStagedBytesAndPropagatesError() async throws { let localBaseItem = makeItem(id: Self.fileID, name: "Report.txt") let latestItem = makeItem( @@ -279,27 +395,32 @@ struct KDriveMutationCoordinatorTests { ]) } - @Test func staleRenameThrowsStaleVersionAndDoesNotRename() async throws { + @Test func concurrentRemoteRenameYieldsToLocalRenameIntent() async throws { let baseItem = makeItem(id: Self.fileID, name: "Old.txt") let latestItem = makeItem( id: Self.fileID, name: "Remote.txt", updatedAt: Date(timeIntervalSince1970: 350) ) - let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem]]) + let renamedItem = makeItem( + id: Self.fileID, + name: "New.txt", + updatedAt: Date(timeIntervalSince1970: 360) + ) + let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem, renamedItem]]) let coordinator = makeCoordinator(remote: remote) - let staleItem = try await expectStaleVersion { - _ = try await coordinator.renameItem( - fileID: Self.fileID, - baseMetadataVersion: baseItem.metadataVersion, - name: "New.txt" - ) - } + let result = try await coordinator.renameItem( + fileID: Self.fileID, + baseMetadataVersion: baseItem.metadataVersion, + name: "New.txt" + ) - #expect(staleItem == latestItem) + #expect(result == renamedItem) #expect(await remote.calls() == [ - .item(driveID: Self.driveID, fileID: Self.fileID) + .item(driveID: Self.driveID, fileID: Self.fileID), + .renameItem(driveID: Self.driveID, fileID: Self.fileID, name: "New.txt"), + .item(driveID: Self.driveID, fileID: Self.fileID), ]) } @@ -418,7 +539,7 @@ struct KDriveMutationCoordinatorTests { ]) } - @Test func movedToDestinationWithUnexpectedNameStillBlocksCombinedMoveRename() async throws { + @Test func combinedMoveRenameAppliesLocalNameAtDestination() async throws { let baseItem = makeItem(id: Self.fileID, name: "Old.txt") let latestItem = makeItem( id: Self.fileID, @@ -426,21 +547,27 @@ struct KDriveMutationCoordinatorTests { parentID: 901, updatedAt: Date(timeIntervalSince1970: 350) ) - let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem]]) + let movedItem = makeItem( + id: Self.fileID, + name: "Moved.txt", + parentID: 901, + updatedAt: Date(timeIntervalSince1970: 360) + ) + let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem, movedItem]]) let coordinator = makeCoordinator(remote: remote) - let staleItem = try await expectStaleVersion { - _ = try await coordinator.moveItem( - fileID: Self.fileID, - baseMetadataVersion: baseItem.metadataVersion, - destinationParentID: 901, - name: "Moved.txt" - ) - } + let result = try await coordinator.moveItem( + fileID: Self.fileID, + baseMetadataVersion: baseItem.metadataVersion, + destinationParentID: 901, + name: "Moved.txt" + ) - #expect(staleItem == latestItem) + #expect(result == movedItem) #expect(await remote.calls() == [ - .item(driveID: Self.driveID, fileID: Self.fileID) + .item(driveID: Self.driveID, fileID: Self.fileID), + .moveItem(driveID: Self.driveID, fileID: Self.fileID, destinationParentID: 901, name: "Moved.txt"), + .item(driveID: Self.driveID, fileID: Self.fileID), ]) } @@ -502,7 +629,7 @@ struct KDriveMutationCoordinatorTests { ]) } - @Test func staleMoveThrowsStaleVersionAndDoesNotMove() async throws { + @Test func concurrentRemoteRenameIsPreservedByMoveOnlyIntent() async throws { let baseItem = makeItem(id: Self.fileID, name: "Old.txt") let latestItem = makeItem( id: Self.fileID, @@ -510,21 +637,27 @@ struct KDriveMutationCoordinatorTests { parentID: 902, updatedAt: Date(timeIntervalSince1970: 350) ) - let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem]]) + let movedItem = makeItem( + id: Self.fileID, + name: "Remote.txt", + parentID: 901, + updatedAt: Date(timeIntervalSince1970: 360) + ) + let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem, movedItem]]) let coordinator = makeCoordinator(remote: remote) - let staleItem = try await expectStaleVersion { - _ = try await coordinator.moveItem( - fileID: Self.fileID, - baseMetadataVersion: baseItem.metadataVersion, - destinationParentID: 901, - name: nil - ) - } + let result = try await coordinator.moveItem( + fileID: Self.fileID, + baseMetadataVersion: baseItem.metadataVersion, + destinationParentID: 901, + name: nil + ) - #expect(staleItem == latestItem) + #expect(result == movedItem) #expect(await remote.calls() == [ - .item(driveID: Self.driveID, fileID: Self.fileID) + .item(driveID: Self.driveID, fileID: Self.fileID), + .moveItem(driveID: Self.driveID, fileID: Self.fileID, destinationParentID: 901, name: nil), + .item(driveID: Self.driveID, fileID: Self.fileID), ]) } @@ -564,7 +697,7 @@ struct KDriveMutationCoordinatorTests { ]) } - @Test func staleTrashThrowsStaleVersionAndDoesNotTrash() async throws { + @Test func trashIntentWinsAfterConcurrentRemoteEdit() async throws { let baseItem = makeItem(id: Self.fileID, name: "Report.txt") let latestItem = makeItem( id: Self.fileID, @@ -575,13 +708,15 @@ struct KDriveMutationCoordinatorTests { let remote = RecordingKDriveFileProvider(itemResults: [Self.fileID: [latestItem]]) let coordinator = makeCoordinator(remote: remote) - let staleItem = try await expectStaleVersion { - _ = try await coordinator.trashItem(fileID: Self.fileID, baseVersion: baseVersion(for: baseItem)) - } + let result = try await coordinator.trashItem( + fileID: Self.fileID, + baseVersion: baseVersion(for: baseItem) + ) - #expect(staleItem == latestItem) + #expect(result == latestItem) #expect(await remote.calls() == [ - .item(driveID: Self.driveID, fileID: Self.fileID) + .item(driveID: Self.driveID, fileID: Self.fileID), + .trashItem(driveID: Self.driveID, fileID: Self.fileID), ]) } @@ -685,7 +820,8 @@ struct KDriveMutationCoordinatorTests { modifiedAt: Date = Date(timeIntervalSince1970: 200), updatedAt: Date = Date(timeIntervalSince1970: 300), type: String? = "file", - mimeType: String? = "text/plain" + mimeType: String? = "text/plain", + etag: String? = nil ) -> KDriveRemoteItem { KDriveRemoteItem( id: id, @@ -699,7 +835,8 @@ struct KDriveMutationCoordinatorTests { mimeType: mimeType, createdAt: Date(timeIntervalSince1970: 100), modifiedAt: modifiedAt, - updatedAt: updatedAt + updatedAt: updatedAt, + etag: etag ?? "etag-\(Int(modifiedAt.timeIntervalSince1970))" ) } @@ -719,6 +856,9 @@ struct KDriveMutationCoordinatorTests { switch error { case .staleVersion(let latestItem): return latestItem + case .localContentConflict(let latestItem, _): + Issue.record("Expected stale metadata version, received local content conflict") + return latestItem } } @@ -735,12 +875,23 @@ private enum RecordingKDriveCall: Equatable, Sendable { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? + ) + case replaceFile( + driveID: Int, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, + contents: Data, + lastModifiedAt: Date? ) - case replaceFile(driveID: Int, parentID: Int, fileName: String, contents: Data, lastModifiedAt: Date?) case createDirectory(driveID: Int, parentID: Int, name: String) case renameItem(driveID: Int, fileID: Int, name: String) case moveItem(driveID: Int, fileID: Int, destinationParentID: Int, name: String?) + case updateModificationDate(driveID: Int, fileID: Int, date: Date) case trashItem(driveID: Int, fileID: Int) case deleteTrashedItem(driveID: Int, fileID: Int) } @@ -761,6 +912,7 @@ private actor RecordingKDriveFileProvider: KDriveFileProviding { private let uploadResult: KDriveRemoteItem? private let uploadError: RecordingKDriveError? private let replaceResult: KDriveRemoteItem? + private let replaceStatusCode: Int? private let directoryResult: KDriveRemoteItem? private var recordedCalls: [RecordingKDriveCall] = [] @@ -769,12 +921,14 @@ private actor RecordingKDriveFileProvider: KDriveFileProviding { uploadResult: KDriveRemoteItem? = nil, uploadError: RecordingKDriveError? = nil, replaceResult: KDriveRemoteItem? = nil, + replaceStatusCode: Int? = nil, directoryResult: KDriveRemoteItem? = nil ) { self.itemResults = itemResults self.uploadResult = uploadResult self.uploadError = uploadError self.replaceResult = replaceResult + self.replaceStatusCode = replaceStatusCode self.directoryResult = directoryResult } @@ -822,7 +976,9 @@ private actor RecordingKDriveFileProvider: KDriveFileProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) async throws -> KDriveRemoteItem { recordedCalls.append(.uploadFile( driveID: driveID, @@ -830,7 +986,9 @@ private actor RecordingKDriveFileProvider: KDriveFileProviding { fileName: fileName, contents: contents, lastModifiedAt: lastModifiedAt, - conflictStrategy: conflictStrategy + conflictStrategy: conflictStrategy, + clientToken: clientToken, + contentHash: contentHash )) if let uploadError { throw uploadError @@ -843,18 +1001,28 @@ private actor RecordingKDriveFileProvider: KDriveFileProviding { func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem { recordedCalls.append(.replaceFile( driveID: driveID, - parentID: parentID, - fileName: fileName, + fileID: fileID, + expectedETag: expectedETag, + clientToken: clientToken, + contentHash: contentHash, contents: contents, lastModifiedAt: lastModifiedAt )) + if let replaceStatusCode { + throw APIClientError.unacceptableStatusCode( + replaceStatusCode, + body: "conditional upload conflict" + ) + } guard let replaceResult else { throw RecordingKDriveError.missingReplaceResult } @@ -882,6 +1050,10 @@ private actor RecordingKDriveFileProvider: KDriveFileProviding { )) } + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { + recordedCalls.append(.updateModificationDate(driveID: driveID, fileID: fileID, date: date)) + } + func trashItem(driveID: Int, fileID: Int) async throws { recordedCalls.append(.trashItem(driveID: driveID, fileID: fileID)) } diff --git a/potassiumProviderTests/KDrivePrivateDirectoryResolverTests.swift b/potassiumProviderTests/KDrivePrivateDirectoryResolverTests.swift index a4ec96b..7337fc0 100644 --- a/potassiumProviderTests/KDrivePrivateDirectoryResolverTests.swift +++ b/potassiumProviderTests/KDrivePrivateDirectoryResolverTests.swift @@ -216,15 +216,19 @@ private actor PrivateDirectoryRemote: KDriveFileProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) async throws -> KDriveRemoteItem { throw PrivateDirectoryRemoteError.unimplemented } func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem { @@ -243,6 +247,10 @@ private actor PrivateDirectoryRemote: KDriveFileProviding { throw PrivateDirectoryRemoteError.unimplemented } + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { + throw PrivateDirectoryRemoteError.unimplemented + } + func trashItem(driveID: Int, fileID: Int) async throws { throw PrivateDirectoryRemoteError.unimplemented } diff --git a/potassiumProviderTests/KnownFolderLifecycleTests.swift b/potassiumProviderTests/KnownFolderLifecycleTests.swift index 567b856..fe9eb05 100644 --- a/potassiumProviderTests/KnownFolderLifecycleTests.swift +++ b/potassiumProviderTests/KnownFolderLifecycleTests.swift @@ -399,15 +399,19 @@ private struct KnownFolderLifecycleRemote: KDriveFileProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) async throws -> KDriveRemoteItem { throw KnownFolderLifecycleTestError.releaseFailed } func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem { @@ -432,6 +436,10 @@ private struct KnownFolderLifecycleRemote: KDriveFileProviding { throw KnownFolderLifecycleTestError.releaseFailed } + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { + throw KnownFolderLifecycleTestError.releaseFailed + } + func trashItem(driveID: Int, fileID: Int) async throws { throw KnownFolderLifecycleTestError.releaseFailed } diff --git a/potassiumProviderTests/WorkingSetSyncTests.swift b/potassiumProviderTests/WorkingSetSyncTests.swift index 5c54bc6..187fa30 100644 --- a/potassiumProviderTests/WorkingSetSyncTests.swift +++ b/potassiumProviderTests/WorkingSetSyncTests.swift @@ -411,11 +411,12 @@ private actor WorkingSetRemoteMock: KDriveFileProviding, KDriveWorkingSetRemoteP func listTrash(driveID: Int, cursor: String?, limit: Int) async throws -> KDriveItemPage { throw WorkingSetRemoteMockError.unimplemented } func downloadFile(driveID: Int, fileID: Int) async throws -> Data { throw WorkingSetRemoteMockError.unimplemented } func thumbnail(driveID: Int, fileID: Int, width: Int?, height: Int?) async throws -> Data { throw WorkingSetRemoteMockError.unimplemented } - func uploadFile(driveID: Int, parentID: Int, fileName: String, contents: Data, lastModifiedAt: Date?, conflictStrategy: KDriveUploadConflictStrategy) async throws -> KDriveRemoteItem { throw WorkingSetRemoteMockError.unimplemented } - func replaceFile(driveID: Int, parentID: Int, fileName: String, contents: Data, lastModifiedAt: Date?) async throws -> KDriveRemoteItem { throw WorkingSetRemoteMockError.unimplemented } + func uploadFile(driveID: Int, parentID: Int, fileName: String, contents: Data, lastModifiedAt: Date?, conflictStrategy: KDriveUploadConflictStrategy, clientToken: String?, contentHash: String?) async throws -> KDriveRemoteItem { throw WorkingSetRemoteMockError.unimplemented } + func replaceFile(driveID: Int, fileID: Int, expectedETag: String, clientToken: String, contentHash: String, contents: Data, lastModifiedAt: Date?) async throws -> KDriveRemoteItem { throw WorkingSetRemoteMockError.unimplemented } func createDirectory(driveID: Int, parentID: Int, name: String) async throws -> KDriveRemoteItem { throw WorkingSetRemoteMockError.unimplemented } func renameItem(driveID: Int, fileID: Int, name: String) async throws { throw WorkingSetRemoteMockError.unimplemented } func moveItem(driveID: Int, fileID: Int, destinationParentID: Int, name: String?) async throws { throw WorkingSetRemoteMockError.unimplemented } + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { throw WorkingSetRemoteMockError.unimplemented } func trashItem(driveID: Int, fileID: Int) async throws { throw WorkingSetRemoteMockError.unimplemented } func deleteTrashedItem(driveID: Int, fileID: Int) async throws { throw WorkingSetRemoteMockError.unimplemented } } diff --git a/potassiumProviderTests/potassiumProviderTests.swift b/potassiumProviderTests/potassiumProviderTests.swift index 6bfd33f..09fc693 100644 --- a/potassiumProviderTests/potassiumProviderTests.swift +++ b/potassiumProviderTests/potassiumProviderTests.swift @@ -1381,10 +1381,19 @@ struct PotassiumProviderCoreTests { #expect(authRejection.recovery == .notAuthenticated) #expect(serverRejection.recovery == .serverUnreachable) #expect(validationRejection.recovery == .cannotSynchronize) + #expect(KDriveRemoteErrorClassifier.isNameCollision( + APIClientError.unacceptableStatusCode(409, body: "Conflict") + )) + #expect(KDriveRemoteErrorClassifier.isNameCollision( + APIClientError.unacceptableStatusCode(422, body: "A file with this name already exists") + )) + #expect(!KDriveRemoteErrorClassifier.isNameCollision( + APIClientError.unacceptableStatusCode(422, body: "Invalid upload parameters") + )) #expect(KDriveRemoteErrorClassifier.apiRejection(from: NSError(domain: NSURLErrorDomain, code: -1009)) == nil) } - @Test func kdriveServiceReplacesFileByNameWithVersionConflictStrategy() async throws { + @Test func kdriveServiceConditionallyReplacesFileByID() async throws { await KDriveJSONRequestCapturingURLProtocol.reset(responseData: Self.fileUploadResponseData) let configuration = URLSessionConfiguration.ephemeral configuration.protocolClasses = [KDriveJSONRequestCapturingURLProtocol.self] @@ -1400,8 +1409,10 @@ struct PotassiumProviderCoreTests { let item = try await service.replaceFile( driveID: 100, - parentID: 7, - fileName: "Edited.jpg", + fileID: 42, + expectedETag: "etag-before", + clientToken: "0123456789abcdef0123456789abcdef", + contentHash: "sha256:abcd", contents: contents, lastModifiedAt: Date(timeIntervalSince1970: 1_700_000_001) ) @@ -1420,12 +1431,16 @@ struct PotassiumProviderCoreTests { #expect(request.value(forHTTPHeaderField: "Authorization") == "Bearer redacted-token") #expect(request.value(forHTTPHeaderField: "Accept") == "application/json") #expect(request.value(forHTTPHeaderField: "Content-Type") == "application/octet-stream") + #expect(request.value(forHTTPHeaderField: "If-Match") == "etag-before") #expect(query["total_size"] == "4") - #expect(query["directory_id"] == "7") - #expect(query["file_name"] == "Edited.jpg") + #expect(query["with"] == "etag") + #expect(query["client_token"] == "0123456789abcdef0123456789abcdef") + #expect(query["total_chunk_hash"] == "sha256:abcd") #expect(query["last_modified_at"] == "1700000001") - #expect(query["conflict"] == "version") - #expect(query["file_id"] == nil) + #expect(query["file_id"] == "42") + #expect(query["directory_id"] == nil) + #expect(query["file_name"] == nil) + #expect(query["conflict"] == nil) } @Test func kdriveServiceCreatesFileWithVersionConflictStrategy() async throws { @@ -2050,7 +2065,8 @@ struct PotassiumProviderCoreTests { modifiedAt: Date = Date(timeIntervalSince1970: 200), updatedAt: Date = Date(timeIntervalSince1970: 300), type: String? = "file", - mimeType: String? = "text/plain" + mimeType: String? = "text/plain", + etag: String? = nil ) -> KDriveRemoteItem { KDriveRemoteItem( id: id, @@ -2064,7 +2080,8 @@ struct PotassiumProviderCoreTests { mimeType: mimeType, createdAt: Date(timeIntervalSince1970: 100), modifiedAt: modifiedAt, - updatedAt: updatedAt + updatedAt: updatedAt, + etag: etag ?? "etag-\(Int(modifiedAt.timeIntervalSince1970))" ) } @@ -2649,15 +2666,19 @@ private struct FakeKDriveFileProvider: KDriveFileProviding { fileName: String, contents: Data, lastModifiedAt: Date?, - conflictStrategy: KDriveUploadConflictStrategy + conflictStrategy: KDriveUploadConflictStrategy, + clientToken: String?, + contentHash: String? ) async throws -> KDriveRemoteItem { throw FakeKDriveFileProviderError.unimplemented } func replaceFile( driveID: Int, - parentID: Int, - fileName: String, + fileID: Int, + expectedETag: String, + clientToken: String, + contentHash: String, contents: Data, lastModifiedAt: Date? ) async throws -> KDriveRemoteItem { @@ -2676,6 +2697,10 @@ private struct FakeKDriveFileProvider: KDriveFileProviding { throw FakeKDriveFileProviderError.unimplemented } + func updateModificationDate(driveID: Int, fileID: Int, date: Date) async throws { + throw FakeKDriveFileProviderError.unimplemented + } + func trashItem(driveID: Int, fileID: Int) async throws { throw FakeKDriveFileProviderError.unimplemented } From bf2ea371f63dd63016f20c3ccf21a34cbb1c3d98 Mon Sep 17 00:00:00 2001 From: OpenCow Date: Sun, 2 Aug 2026 14:22:24 +0200 Subject: [PATCH 2/2] fix: persist kdrive content revisions --- .../SQLiteSnapshotStore.swift | 36 +++++++++++++++-- doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md | 14 +++++-- doc/LISTING_AND_VERSIONING.md | 3 +- doc/PERSISTENCE.md | 16 +++++--- potassiumProvider.xcodeproj/project.pbxproj | 4 +- .../xcshareddata/swiftpm/Package.resolved | 3 +- .../potassiumProviderTests.swift | 40 ++++++++++++++++++- 7 files changed, 96 insertions(+), 20 deletions(-) diff --git a/PotassiumProviderCore/SQLiteSnapshotStore.swift b/PotassiumProviderCore/SQLiteSnapshotStore.swift index 7fa8f57..c4a9d49 100644 --- a/PotassiumProviderCore/SQLiteSnapshotStore.swift +++ b/PotassiumProviderCore/SQLiteSnapshotStore.swift @@ -14,7 +14,7 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta let database = try Connection(databaseURL.path) try Self.configure(database) try Self.createTables(on: database) - try Self.migrateFavoriteMetadata(on: database) + try Self.migrateItemMetadataColumns(on: database) try Self.migrateLegacySnapshots(on: database) self.database = database } @@ -865,7 +865,9 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta table.column(Schema.isFavorite) table.column(Schema.createdAt) table.column(Schema.modifiedAt) + table.column(Schema.revisedAt) table.column(Schema.itemUpdatedAt) + table.column(Schema.etag) table.primaryKey(Schema.domainIdentifier, Schema.containerIdentifier, Schema.itemID) }) @@ -909,7 +911,9 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta table.column(GenerationSchema.isFavorite) table.column(GenerationSchema.createdAt) table.column(GenerationSchema.modifiedAt) + table.column(GenerationSchema.revisedAt) table.column(GenerationSchema.itemUpdatedAt) + table.column(GenerationSchema.etag) table.primaryKey( GenerationSchema.domainIdentifier, GenerationSchema.containerIdentifier, @@ -952,13 +956,25 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta }) } - private static func migrateFavoriteMetadata(on database: Connection) throws { + private static func migrateItemMetadataColumns(on database: Connection) throws { if try tableHasColumn("isFavorite", table: "snapshot_items", database: database) == false { try database.run(Schema.snapshotItems.addColumn(Schema.isFavorite)) } + if try tableHasColumn("revisedAt", table: "snapshot_items", database: database) == false { + try database.run(Schema.snapshotItems.addColumn(Schema.revisedAt)) + } + if try tableHasColumn("etag", table: "snapshot_items", database: database) == false { + try database.run(Schema.snapshotItems.addColumn(Schema.etag)) + } if try tableHasColumn("isFavorite", table: "snapshot_generation_items", database: database) == false { try database.run(GenerationSchema.items.addColumn(GenerationSchema.isFavorite)) } + if try tableHasColumn("revisedAt", table: "snapshot_generation_items", database: database) == false { + try database.run(GenerationSchema.items.addColumn(GenerationSchema.revisedAt)) + } + if try tableHasColumn("etag", table: "snapshot_generation_items", database: database) == false { + try database.run(GenerationSchema.items.addColumn(GenerationSchema.etag)) + } } private static func tableHasColumn( @@ -1080,7 +1096,9 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta Schema.isFavorite <- item.isFavorite, Schema.createdAt <- item.createdAt?.timeIntervalSince1970, Schema.modifiedAt <- item.modifiedAt.timeIntervalSince1970, + Schema.revisedAt <- item.revisedAt?.timeIntervalSince1970, Schema.itemUpdatedAt <- item.updatedAt.timeIntervalSince1970, + Schema.etag <- item.etag, ] } @@ -1108,7 +1126,9 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta GenerationSchema.isFavorite <- item.isFavorite, GenerationSchema.createdAt <- item.createdAt?.timeIntervalSince1970, GenerationSchema.modifiedAt <- item.modifiedAt.timeIntervalSince1970, + GenerationSchema.revisedAt <- item.revisedAt?.timeIntervalSince1970, GenerationSchema.itemUpdatedAt <- item.updatedAt.timeIntervalSince1970, + GenerationSchema.etag <- item.etag, ] } @@ -1126,7 +1146,9 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta isFavorite: row[Schema.isFavorite], createdAt: row[Schema.createdAt].map { Date(timeIntervalSince1970: $0) }, modifiedAt: Date(timeIntervalSince1970: row[Schema.modifiedAt]), - updatedAt: Date(timeIntervalSince1970: row[Schema.itemUpdatedAt]) + revisedAt: row[Schema.revisedAt].map { Date(timeIntervalSince1970: $0) }, + updatedAt: Date(timeIntervalSince1970: row[Schema.itemUpdatedAt]), + etag: row[Schema.etag] ) } @@ -1144,7 +1166,9 @@ public actor KDriveSnapshotSQLiteStore: KDriveSnapshotStoring, KDriveSnapshotSta isFavorite: row[GenerationSchema.isFavorite], createdAt: row[GenerationSchema.createdAt].map { Date(timeIntervalSince1970: $0) }, modifiedAt: Date(timeIntervalSince1970: row[GenerationSchema.modifiedAt]), - updatedAt: Date(timeIntervalSince1970: row[GenerationSchema.itemUpdatedAt]) + revisedAt: row[GenerationSchema.revisedAt].map { Date(timeIntervalSince1970: $0) }, + updatedAt: Date(timeIntervalSince1970: row[GenerationSchema.itemUpdatedAt]), + etag: row[GenerationSchema.etag] ) } @@ -1267,7 +1291,9 @@ private enum Schema { static let isFavorite = Expression("isFavorite") static let createdAt = Expression("createdAt") static let modifiedAt = Expression("modifiedAt") + static let revisedAt = Expression("revisedAt") static let itemUpdatedAt = Expression("itemUpdatedAt") + static let etag = Expression("etag") } private enum GenerationSchema { @@ -1298,7 +1324,9 @@ private enum GenerationSchema { static let isFavorite = Expression("isFavorite") static let createdAt = Expression("createdAt") static let modifiedAt = Expression("modifiedAt") + static let revisedAt = Expression("revisedAt") static let itemUpdatedAt = Expression("itemUpdatedAt") + static let etag = Expression("etag") } private enum WorkingSetSchema { diff --git a/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md b/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md index 9a2fa6f..c4e7566 100644 --- a/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md +++ b/doc/CONFLICT_RESOLUTION_TRUTH_TABLE.md @@ -24,9 +24,13 @@ audited truth table takes precedence and the inconsistency must be corrected. - iOS Simulator app build passed on iPhone 17 / iOS 26.5 with code signing disabled - generic visionOS app build passed with code signing disabled - - Full macOS unit run: conflict/version/recovery tests passed; three existing - snapshot-store tests were flaky under parallel execution and the full - scheme UI runner could not finish because the host disk was full + - The first GitHub macOS run exposed that snapshot persistence omitted the + new ETag/revision fields; the schema, migration, and round-trip regression + have been updated. macOS `build-for-testing` passed against merged + `potassiumChannel` revision `81014d3`; the GitHub test rerun was pending at + this audit commit. + - The local full-scheme UI runner could not finish because the host disk was + full; focused tests avoid that environment-specific runner failure. - Live kDrive collision validation: not performed; server-dependent behavior is identified explicitly below - Finding state vocabulary: **Open**, **Mitigated**, or **Resolved** @@ -47,6 +51,8 @@ The table is derived from these implementation boundaries: compares base versions and selects mutation or conflict-copy behavior. - [`KDriveVersionConflictResolver`](../PotassiumProviderCore/KDriveModels.swift) defines content and metadata equality. +- [`KDriveSnapshotSQLiteStore`](../PotassiumProviderCore/SQLiteSnapshotStore.swift) + persists ETags and revisions across cached enumeration and process restarts. - [`PotassiumKDriveService`](../PotassiumProviderCore/KDriveRemoteService.swift) selects kDrive conflict flags and constructs requests. - [`FileProviderRuntime`](../potassiumProviderFileProvider/FileProviderRuntime.swift) @@ -147,7 +153,7 @@ pending and are never falsely acknowledged. | `CR-001` | Critical | Combined `changedFields` were mutually exclusive and falsely reported complete. | The implementation now applies move/rename, content/date, and trash in order and returns unsupported fields pending. End-to-end extension callback coverage is still required. | **Mitigated** | | `CR-002` | High | Existing-item mutations had a fetch-then-mutate race. | Content now uses ETag/`If-Match`; permanent delete still lacks a documented conditional server primitive. | **Mitigated** | | `CR-003` | High | Content replacement addressed latest parent/name instead of stable ID. | Replacement now uses `file_id`, authoritative ETag, and `If-Match`; conditional races preserve both. | **Resolved** | -| `CR-004` | High | Content versions used only `modifiedAt`. | Versions now contain stable item ID plus ETag; legacy/missing ETags fail closed. | **Resolved** | +| `CR-004` | High | Content versions used only `modifiedAt`, and the first ETag implementation did not persist ETags through SQLite snapshot round trips. | Versions now contain stable item ID plus ETag; snapshot schemas and in-place migrations retain ETag/revision metadata; legacy/missing ETags fail closed. | **Resolved** | | `CR-005` | High | Latest lookup happened before staging. | Bytes now stage first, but a preflight failure may leave an unindexed recovery copy because parent metadata is unavailable. | **Mitigated** | | `CR-006` | High | Contents+trash ignored the new bytes. | Content is replaced/preserved before trash; conflict item and original are both trashed when required. | **Resolved** | | `CR-007` | Medium | Failed uploads stranded private staged bytes. | Indexed failures have Activities reveal/export and deterministic replay; provider-owned scheduling and Retry Now remain absent. | **Mitigated** | diff --git a/doc/LISTING_AND_VERSIONING.md b/doc/LISTING_AND_VERSIONING.md index 0a1b6d9..6d6275a 100644 --- a/doc/LISTING_AND_VERSIONING.md +++ b/doc/LISTING_AND_VERSIONING.md @@ -209,7 +209,8 @@ stale writer does not overwrite newer cache state. - `serverCursor`: kDrive advanced listing cursor for normal folders - `isFullyEnumerated`: whether all pages have been fetched - `usesAdvancedListing`: whether this snapshot came from advanced listing -- `items`: cached `KDriveRemoteItem` metadata +- `items`: cached `KDriveRemoteItem` metadata, including nullable ETag and + `revisedAt`; older rows without ETags fail closed until refreshed ## Item Version Mapping diff --git a/doc/PERSISTENCE.md b/doc/PERSISTENCE.md index c3ff0da..0ad7d99 100644 --- a/doc/PERSISTENCE.md +++ b/doc/PERSISTENCE.md @@ -86,8 +86,8 @@ Listing snapshots use three active tables: - commit timestamp `snapshot_generation_items` stores the ordered item metadata for a particular -generation, including nullable `isFavorite` state. Its primary key is domain, -container, generation, and item ID. +generation, including nullable `isFavorite`, `revisedAt`, and authoritative +ETag state. Its primary key is domain, container, generation, and item ID. The active generation and its two predecessors are retained. That keeps item and change page tokens stable while a newer snapshot commits, while deliberately @@ -98,9 +98,10 @@ Initialization transactionally moves each legacy container into generation 1 and deletes the migrated legacy rows without changing the working-set, conflict, or activity tables: -Before that migration, initialization adds nullable `isFavorite` columns to -both legacy and generation item tables when upgrading an older database. -Existing rows remain `NULL`, preserving backward compatibility. +Before that migration, initialization adds nullable `isFavorite`, `revisedAt`, +and `etag` columns to both legacy and generation item tables when upgrading an +older database. Existing rows remain `NULL`, preserving backward compatibility +and causing version checks to fail closed until those items are refreshed. `container_snapshots`: @@ -129,7 +130,9 @@ Existing rows remain `NULL`, preserving backward compatibility. - nullable `isFavorite` - `createdAt` - `modifiedAt` +- nullable `revisedAt` - `itemUpdatedAt` +- nullable `etag` New writes use only the generation tables. Domain cleanup removes both legacy and generation rows. @@ -227,7 +230,8 @@ SQLite caches metadata needed to enumerate and diff containers: - type/status - size and MIME type - nullable kDrive favorite state -- timestamps used for File Provider versions +- ETag and revision timestamps used for authoritative File Provider content + versions - advanced-listing cursor state - whether the container has been fully enumerated - conflict/audit metadata needed by the app's Activities tab diff --git a/potassiumProvider.xcodeproj/project.pbxproj b/potassiumProvider.xcodeproj/project.pbxproj index 7dca86a..5cc655d 100644 --- a/potassiumProvider.xcodeproj/project.pbxproj +++ b/potassiumProvider.xcodeproj/project.pbxproj @@ -1255,8 +1255,8 @@ isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/OpenCow42/potassiumChannel.git"; requirement = { - branch = "codex/kdrive-etag-revisions"; - kind = branch; + kind = revision; + revision = 81014d32428b2f367c74c7f1616793c7a5b2ba01; }; }; C0DCD8ED2FFA44AB00520215 /* XCRemoteSwiftPackageReference "swift-concurrency" */ = { diff --git a/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 9ed49fb..4ce278e 100644 --- a/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -6,8 +6,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/OpenCow42/potassiumChannel.git", "state" : { - "branch" : "codex/kdrive-etag-revisions", - "revision" : "1bed1613782e3d0a6f707e3ddc6ebda5e0bddec5" + "revision" : "81014d32428b2f367c74c7f1616793c7a5b2ba01" } }, { diff --git a/potassiumProviderTests/potassiumProviderTests.swift b/potassiumProviderTests/potassiumProviderTests.swift index 09fc693..840f92a 100644 --- a/potassiumProviderTests/potassiumProviderTests.swift +++ b/potassiumProviderTests/potassiumProviderTests.swift @@ -228,7 +228,12 @@ struct PotassiumProviderCoreTests { serverCursor: "root-cursor", isFullyEnumerated: true, usesAdvancedListing: true, - items: [makeItem(id: 1, name: "Root.txt")] + items: [makeItem( + id: 1, + name: "Root.txt", + revisedAt: Date(timeIntervalSince1970: 250), + etag: "root-etag" + )] ) let trashSnapshot = KDriveSnapshot(anchor: "trash-anchor", items: [makeItem(id: 2, name: "Trash.txt")]) @@ -249,6 +254,37 @@ struct PotassiumProviderCoreTests { #expect(try await store.snapshot(domainIdentifier: "domain/1", containerIdentifier: "trash") == nil) } + @Test func snapshotStoreMigratesMissingRemoteVersionColumns() async throws { + let directory = temporaryDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let databaseURL = directory.appendingPathComponent("Snapshots.sqlite3") + + // Create the complete pre-migration table set, then model a database + // written by the previous provider version before ETag persistence. + _ = try KDriveSnapshotSQLiteStore(databaseURL: databaseURL) + do { + let database = try Connection(databaseURL.path) + try database.execute("ALTER TABLE snapshot_items DROP COLUMN revisedAt") + try database.execute("ALTER TABLE snapshot_items DROP COLUMN etag") + try database.execute("ALTER TABLE snapshot_generation_items DROP COLUMN revisedAt") + try database.execute("ALTER TABLE snapshot_generation_items DROP COLUMN etag") + } + + let store = try KDriveSnapshotSQLiteStore(databaseURL: databaseURL) + let snapshot = KDriveSnapshot( + anchor: "migrated-anchor", + items: [makeItem( + id: 9, + name: "Migrated.txt", + revisedAt: Date(timeIntervalSince1970: 275), + etag: "migrated-etag" + )] + ) + try await store.save(snapshot, domainIdentifier: "domain-1", containerIdentifier: "root") + + #expect(try await store.snapshot(domainIdentifier: "domain-1", containerIdentifier: "root") == snapshot) + } + @Test func snapshotStoreReportsDomainStatistics() async throws { let directory = temporaryDirectory() defer { try? FileManager.default.removeItem(at: directory) } @@ -2063,6 +2099,7 @@ struct PotassiumProviderCoreTests { name: String, parentID: Int = ProviderConstants.defaultRootFileID, modifiedAt: Date = Date(timeIntervalSince1970: 200), + revisedAt: Date? = nil, updatedAt: Date = Date(timeIntervalSince1970: 300), type: String? = "file", mimeType: String? = "text/plain", @@ -2080,6 +2117,7 @@ struct PotassiumProviderCoreTests { mimeType: mimeType, createdAt: Date(timeIntervalSince1970: 100), modifiedAt: modifiedAt, + revisedAt: revisedAt, updatedAt: updatedAt, etag: etag ?? "etag-\(Int(modifiedAt.timeIntervalSince1970))" )