diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index b483ece637b49..32b7c92b3ce38 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -463,9 +463,6 @@ public final class FilesDatabaseManager: Sendable { result.downloaded = false } else if result.isUpload { result.uploaded = false - result.chunkUploadId = UUID().uuidString - } else if status == .normal, metadata.isUpload { - result.chunkUploadId = nil } logger.debug("Updated status for item metadata.", [ diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift index 5af0c6459d0f6..02398e9fc5e1b 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift @@ -137,15 +137,13 @@ public extension Item { log: any FileProviderLogging ) async -> (Item?, Error?) { let logger = FileProviderLogger(category: "Item", log: log) - let chunkUploadId = - itemTemplate.itemIdentifier.rawValue.replacingOccurrences(of: "/", with: "") let (ocId, etag, date, size, error) = await upload( fileLocatedAt: localPath, toRemotePath: remotePath, usingRemoteInterface: remoteInterface, withAccount: account, inChunksSized: forcedChunkSize, - usingChunkUploadId: chunkUploadId, + forItemWithIdentifier: itemTemplate.itemIdentifier.rawValue, dbManager: dbManager, creationDate: itemTemplate.creationDate as? Date, modificationDate: itemTemplate.contentModificationDate as? Date, diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift index 258066d8aa89c..665b06e0f6661 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift @@ -139,7 +139,7 @@ public extension Item { usingRemoteInterface: remoteInterface, withAccount: account, inChunksSized: forcedChunkSize, - usingChunkUploadId: metadata.chunkUploadId, + forItemWithIdentifier: ocId, dbManager: dbManager, creationDate: newCreationDate, modificationDate: newContentModificationDate, diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Utilities/Upload.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Utilities/Upload.swift index 4e82d7b00ae69..a9c55c9d53b96 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Utilities/Upload.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Utilities/Upload.swift @@ -9,13 +9,44 @@ import RealmSwift let defaultFileChunkSize = 104_857_600 // 100 MiB +/// The per-item prefix shared by every chunked-upload identifier for a given item, so stale chunk +/// bookkeeping from earlier versions of the same item can be swept. +func chunkUploadIdentifierPrefix(forItemWithIdentifier itemIdentifier: String) -> String { + itemIdentifier.replacingOccurrences(of: "/", with: "") + "_" +} + +/// Derives a stable, *content-scoped* identifier for a chunked upload's server folder and its local +/// `RemoteFileChunk` bookkeeping. +/// +/// The identity is `(item, size, modificationDate)`: the same content re-uploads under the same id, +/// so an interrupted transfer resumes and reuses the chunks already stored on the server. Any content +/// change yields a different id, so chunks from a previous version are never spliced into a different +/// one (see F3). The value is prefixed with the item id so stale sets can be swept by prefix. +/// +/// When no modification date is available the content can't be bound to an id, so a per-attempt unique +/// id is used: this forgoes resume for that upload but never risks a bad splice. In the File Provider +/// model a real content change always bumps `contentModificationDate` (that is how the change was +/// detected), so equal `(size, modificationDate)` reliably means identical content. +func chunkUploadIdentifier( + forItemWithIdentifier itemIdentifier: String, fileSize: Int64, modificationDate: Date? +) -> String { + let prefix = chunkUploadIdentifierPrefix(forItemWithIdentifier: itemIdentifier) + + guard let modificationDate else { + return prefix + UUID().uuidString + } + + let mtimeSeconds = Int64(modificationDate.timeIntervalSince1970.rounded()) + return "\(prefix)\(fileSize)_\(mtimeSeconds)" +} + func upload( fileLocatedAt localFilePath: String, toRemotePath remotePath: String, usingRemoteInterface remoteInterface: RemoteInterface, withAccount account: Account, inChunksSized chunkSize: Int? = nil, - usingChunkUploadId chunkUploadId: String? = UUID().uuidString, + forItemWithIdentifier itemIdentifier: String, dbManager: FilesDatabaseManager, creationDate: Date? = nil, modificationDate: Date? = nil, @@ -106,7 +137,9 @@ func upload( return (ocId, etag, date as? Date, size, remoteError) } - let chunkUploadId = chunkUploadId ?? UUID().uuidString + let chunkUploadId = chunkUploadIdentifier( + forItemWithIdentifier: itemIdentifier, fileSize: fileSize, modificationDate: modificationDate + ) uploadLogger.info( """ @@ -117,6 +150,26 @@ func upload( """ ) + // Content-scoped resume (F3): drop any chunk bookkeeping left over from a *different* version of + // this same item — a prior interrupted upload whose content has since changed derives a different + // id. Keeping those rows would risk resuming against server-side chunks that belong to the old + // content and splicing them into the new file. Rows under the current id (a genuine resume of + // identical content) share the id and are preserved. + let staleChunkPrefix = chunkUploadIdentifierPrefix(forItemWithIdentifier: itemIdentifier) + do { + let db = dbManager.ncDatabase() + let staleChunks = db.objects(RemoteFileChunk.self).where { + $0.remoteChunkStoreFolderName.starts(with: staleChunkPrefix) + && $0.remoteChunkStoreFolderName != chunkUploadId + } + + if !staleChunks.isEmpty { + try db.write { db.delete(staleChunks) } + } + } catch { + uploadLogger.error("Could not clear stale chunk bookkeeping for item.", [.error: error]) + } + let remainingChunks = dbManager .ncDatabase() .objects(RemoteFileChunk.self) diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift index d2aef551219ec..b1f7f665d8796 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift @@ -697,13 +697,31 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { func testCreateFileChunkedResumed() async throws { let chunkSize = 2 - let expectedChunkUploadId = UUID().uuidString // Check if illegal characters are stripped - let illegalChunkUploadId = expectedChunkUploadId + "/" // Check if illegal characters are stripped + let expectedChunkUploadIdBase = UUID().uuidString // Check that illegal characters are stripped. + let illegalChunkUploadId = expectedChunkUploadIdBase + "/" // Check that illegal characters are stripped. + + let tempUrl = FileManager.default.temporaryDirectory.appendingPathComponent("file") + let tempData = Data(repeating: 1, count: chunkSize * 3) + try tempData.write(to: tempUrl) + + // New-item creation has no persisted ItemMetadata (the OS supplies the template), so the chunk + // id is derived from the template's itemIdentifier plus the content's (size, modificationDate), + // with illegal path characters stripped. Seed the prior interrupted attempt under exactly that + // derived id so the resume path recognises identical content. + let modificationDate = Date(timeIntervalSince1970: 1_700_000_000) + let chunkUploadId = chunkUploadIdentifier( + forItemWithIdentifier: illegalChunkUploadId, + fileSize: Int64(tempData.count), + modificationDate: modificationDate + ) + XCTAssertTrue(chunkUploadId.hasPrefix(expectedChunkUploadIdBase + "_")) + XCTAssertFalse(chunkUploadId.contains("/")) + let previousUploadedChunkNum = 1 let preexistingChunk = RemoteFileChunk( fileName: String(previousUploadedChunkNum), size: Int64(chunkSize), - remoteChunkStoreFolderName: expectedChunkUploadId + remoteChunkStoreFolderName: chunkUploadId ) let db = Self.dbManager.ncDatabase() @@ -712,37 +730,24 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { RemoteFileChunk( fileName: String(previousUploadedChunkNum + 1), size: Int64(chunkSize), - remoteChunkStoreFolderName: expectedChunkUploadId + remoteChunkStoreFolderName: chunkUploadId ), RemoteFileChunk( fileName: String(previousUploadedChunkNum + 2), size: Int64(chunkSize), - remoteChunkStoreFolderName: expectedChunkUploadId + remoteChunkStoreFolderName: chunkUploadId ) ]) } let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) - remoteInterface.currentChunks = [expectedChunkUploadId: [preexistingChunk]] - - // With real new item uploads we do not have an associated ItemMetadata as the template is - // passed onto us by the OS. We cannot rely on the chunkUploadId property we usually use - // during modified item uploads. - // - // We therefore can only use the system-provided item template's itemIdentifier as the - // chunked upload identifier during new item creation. - // - // To test this situation we set the ocId of the metadata used to construct the item - // template to the chunk upload id. + remoteInterface.currentChunks = [chunkUploadId: [preexistingChunk]] + var fileItemMetadata = SendableItemMetadata( ocId: illegalChunkUploadId, fileName: "file", account: Self.account ) - fileItemMetadata.ocId = illegalChunkUploadId fileItemMetadata.classFile = NKTypeClassFile.document.rawValue - - let tempUrl = FileManager.default.temporaryDirectory.appendingPathComponent("file") - let tempData = Data(repeating: 1, count: chunkSize * 3) - try tempData.write(to: tempUrl) + fileItemMetadata.date = modificationDate let fileItemTemplate = Item( metadata: fileItemMetadata, @@ -776,7 +781,7 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { XCTAssertEqual(remoteItem.directory, fileItemMetadata.directory) XCTAssertEqual(remoteItem.data, tempData) XCTAssertEqual( - remoteInterface.completedChunkTransferSize[expectedChunkUploadId], + remoteInterface.completedChunkTransferSize[chunkUploadId], Int64(tempData.count) - preexistingChunk.size ) diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift index 70b5cc555591f..a9b981084fd51 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift @@ -1292,13 +1292,31 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { func testModifyFileContentsChunkedResumed() async throws { let chunkSize = 2 - let chunkUploadId = UUID().uuidString + let newContents = Data(repeating: 1, count: chunkSize * 3) + let newContentsUrl = FileManager.default.temporaryDirectory.appendingPathComponent("test") + try newContents.write(to: newContentsUrl) + + let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) + + let itemMetadata = remoteItem.toItemMetadata(account: Self.account) + Self.dbManager.addItemMetadata(itemMetadata) + + // The chunk id is derived from (item, size, modificationDate); seed the prior interrupted + // attempt under exactly that derived id so the resume path recognises identical content. + let modificationDate = Date(timeIntervalSince1970: 1_700_000_000) + let chunkUploadId = chunkUploadIdentifier( + forItemWithIdentifier: itemMetadata.ocId, + fileSize: Int64(newContents.count), + modificationDate: modificationDate + ) + let previousUploadedChunkNum = 1 let preexistingChunk = RemoteFileChunk( fileName: String(previousUploadedChunkNum), size: Int64(chunkSize), remoteChunkStoreFolderName: chunkUploadId ) + remoteInterface.currentChunks = [chunkUploadId: [preexistingChunk]] let db = Self.dbManager.ncDatabase() try db.write { @@ -1316,19 +1334,8 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { ]) } - let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) - remoteInterface.currentChunks = [chunkUploadId: [preexistingChunk]] - - var itemMetadata = remoteItem.toItemMetadata(account: Self.account) - itemMetadata.chunkUploadId = chunkUploadId - Self.dbManager.addItemMetadata(itemMetadata) - - let newContents = Data(repeating: 1, count: chunkSize * 3) - let newContentsUrl = FileManager.default.temporaryDirectory.appendingPathComponent("test") - try newContents.write(to: newContentsUrl) - var targetItemMetadata = SendableItemMetadata(value: itemMetadata) - targetItemMetadata.date = .init() + targetItemMetadata.date = modificationDate targetItemMetadata.size = Int64(newContents.count) let item = Item( @@ -1366,9 +1373,6 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { remoteInterface.completedChunkTransferSize[chunkUploadId], Int64(newContents.count) - preexistingChunk.size ) - - let dbItem = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: itemMetadata.ocId)) - XCTAssertNil(dbItem.chunkUploadId) } func testModifyDoesNotPropagateIgnoredFile() async { diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/UploadTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/UploadTests.swift index b9f7ff8536ef5..480abb97116bb 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/UploadTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/UploadTests.swift @@ -31,6 +31,7 @@ final class UploadTests: NextcloudFileProviderKitTestCase { toRemotePath: remotePath, usingRemoteInterface: remoteInterface, withAccount: Self.account, + forItemWithIdentifier: "standard-upload-item", dbManager: Self.dbManager, log: FileProviderLogMock() ) @@ -58,6 +59,7 @@ final class UploadTests: NextcloudFileProviderKitTestCase { usingRemoteInterface: remoteInterface, withAccount: Self.account, inChunksSized: chunkSize, + forItemWithIdentifier: "chunked-upload-item", dbManager: Self.dbManager, log: FileProviderLogMock(), chunkUploadCompleteHandler: { uploadedChunks.append($0) } @@ -90,15 +92,24 @@ final class UploadTests: NextcloudFileProviderKitTestCase { let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: MockRemoteItem.rootItem(account: Self.account)) let chunkSize = 3 - let uploadUuid = UUID().uuidString + + // The chunk id is derived from (item, size, modificationDate). Seed the prior attempt's + // bookkeeping under exactly that derived id so the resume path recognises identical content. + let itemIdentifier = "resume-item" + let modificationDate = Date(timeIntervalSince1970: 1_700_000_000) + let uploadId = chunkUploadIdentifier( + forItemWithIdentifier: itemIdentifier, + fileSize: Int64(data.count), + modificationDate: modificationDate + ) + let previousUploadedChunkNum = 1 let previousUploadedChunk = RemoteFileChunk( fileName: String(previousUploadedChunkNum), size: Int64(chunkSize), - remoteChunkStoreFolderName: uploadUuid + remoteChunkStoreFolderName: uploadId ) - let previousUploadedChunks = [previousUploadedChunk] - remoteInterface.currentChunks = [uploadUuid: previousUploadedChunks] + remoteInterface.currentChunks = [uploadId: [previousUploadedChunk]] let db = Self.dbManager.ncDatabase() try db.write { @@ -106,12 +117,12 @@ final class UploadTests: NextcloudFileProviderKitTestCase { RemoteFileChunk( fileName: String(previousUploadedChunkNum + 1), size: Int64(chunkSize), - remoteChunkStoreFolderName: uploadUuid + remoteChunkStoreFolderName: uploadId ), RemoteFileChunk( fileName: String(previousUploadedChunkNum + 2), size: Int64(data.count - (chunkSize * (previousUploadedChunkNum + 1))), - remoteChunkStoreFolderName: uploadUuid + remoteChunkStoreFolderName: uploadId ) ]) } @@ -124,8 +135,9 @@ final class UploadTests: NextcloudFileProviderKitTestCase { usingRemoteInterface: remoteInterface, withAccount: Self.account, inChunksSized: chunkSize, - usingChunkUploadId: uploadUuid, + forItemWithIdentifier: itemIdentifier, dbManager: Self.dbManager, + modificationDate: modificationDate, log: FileProviderLogMock(), chunkUploadCompleteHandler: { uploadedChunks.append($0) } ) @@ -135,13 +147,13 @@ final class UploadTests: NextcloudFileProviderKitTestCase { XCTAssertNotNil(result.ocId) XCTAssertNotNil(result.etag) + // Only the not-yet-uploaded chunks (2 and 3) are re-sent; chunk 1 is resumed from the server. let firstUploadedChunk = try XCTUnwrap(uploadedChunks.first) let firstUploadedChunkNameInt = try XCTUnwrap(Int(firstUploadedChunk.fileName)) let lastUploadedChunk = try XCTUnwrap(uploadedChunks.last) let lastUploadedChunkNameInt = try XCTUnwrap(Int(lastUploadedChunk.fileName)) XCTAssertEqual(firstUploadedChunkNameInt, previousUploadedChunkNum + 1) XCTAssertEqual(lastUploadedChunkNameInt, previousUploadedChunkNum + 2) - print(uploadedChunks) XCTAssertEqual(Int(firstUploadedChunk.size), chunkSize) XCTAssertEqual( Int(lastUploadedChunk.size), data.count - ((lastUploadedChunkNameInt - 1) * chunkSize) @@ -220,6 +232,7 @@ final class UploadTests: NextcloudFileProviderKitTestCase { toRemotePath: remotePath, usingRemoteInterface: remoteInterface, withAccount: Self.account, + forItemWithIdentifier: "caps-chunk-size-item", dbManager: Self.dbManager, log: FileProviderLogMock(), chunkUploadCompleteHandler: { uploadedChunks.append($0) } @@ -297,6 +310,7 @@ final class UploadTests: NextcloudFileProviderKitTestCase { toRemotePath: remotePath, usingRemoteInterface: remoteInterface, withAccount: Self.account, + forItemWithIdentifier: "caps-no-chunk-size-item", dbManager: Self.dbManager, log: FileProviderLogMock(), chunkUploadCompleteHandler: { uploadedChunks.append($0) } @@ -310,4 +324,70 @@ final class UploadTests: NextcloudFileProviderKitTestCase { XCTAssertEqual(uploadedChunks.first?.size, Int64(defaultFileChunkSize)) XCTAssertEqual(uploadedChunks.last?.size, 1) } + + /// F3 content-safety: a prior interrupted chunked upload of a *different* version of the same item + /// (different derived id) must NOT be resumed. Its stale chunk bookkeeping is swept and the upload + /// starts fresh, so old chunks can never be spliced into the new content. + func testChunkedUploadDiscardsStaleChunksAfterContentChange() async throws { + let fileUrl = + FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + let data = Data(repeating: 1, count: 8) + try data.write(to: fileUrl) + + let remoteInterface = + MockRemoteInterface(account: Self.account, rootItem: MockRemoteItem.rootItem(account: Self.account)) + let chunkSize = 3 + let itemIdentifier = "content-change-item" + + // Seed bookkeeping from a prior interrupted upload of an older version (older mtime). + let staleModificationDate = Date(timeIntervalSince1970: 1_600_000_000) + let staleUploadId = chunkUploadIdentifier( + forItemWithIdentifier: itemIdentifier, + fileSize: Int64(data.count), + modificationDate: staleModificationDate + ) + let db = Self.dbManager.ncDatabase() + try db.write { + db.add([ + RemoteFileChunk(fileName: "2", size: Int64(chunkSize), remoteChunkStoreFolderName: staleUploadId), + RemoteFileChunk(fileName: "3", size: 2, remoteChunkStoreFolderName: staleUploadId) + ]) + } + + // The newer version (different mtime) derives a different id. + let newModificationDate = Date(timeIntervalSince1970: 1_700_000_000) + let newUploadId = chunkUploadIdentifier( + forItemWithIdentifier: itemIdentifier, + fileSize: Int64(data.count), + modificationDate: newModificationDate + ) + XCTAssertNotEqual(staleUploadId, newUploadId) + + let remotePath = Self.account.davFilesUrl + "/file.txt" + var uploadedChunks = [RemoteFileChunk]() + let result = await NextcloudFileProviderKit.upload( + fileLocatedAt: fileUrl.path, + toRemotePath: remotePath, + usingRemoteInterface: remoteInterface, + withAccount: Self.account, + inChunksSized: chunkSize, + forItemWithIdentifier: itemIdentifier, + dbManager: Self.dbManager, + modificationDate: newModificationDate, + log: FileProviderLogMock(), + chunkUploadCompleteHandler: { uploadedChunks.append($0) } + ) + + XCTAssertEqual(result.remoteError, .success) + + // Stale rows for the previous version must have been swept. + let dbAfterUpload = Self.dbManager.ncDatabase() + let remainingStale = dbAfterUpload.objects(RemoteFileChunk.self) + .where { $0.remoteChunkStoreFolderName == staleUploadId } + XCTAssertEqual(remainingStale.count, 0) + + // The upload started fresh (chunk 1 was re-sent, not resumed from chunk 2). + let firstUploadedChunk = try XCTUnwrap(uploadedChunks.first) + XCTAssertEqual(Int(firstUploadedChunk.fileName), 1) + } }