From bab9ce5ce0afe1714e5d55fad0c217efcc32a471 Mon Sep 17 00:00:00 2001 From: Philipp Schmid Date: Fri, 3 Apr 2026 19:20:59 +0200 Subject: [PATCH 1/2] Add test to check hypothesis --- .../FileProviderAdapterMoveItemTests.swift | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift b/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift index d24d6bea0..642ec37fe 100644 --- a/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift +++ b/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift @@ -128,6 +128,51 @@ class FileProviderAdapterMoveItemTests: FileProviderAdapterTestCase { XCTAssertEqual(targetCloudPath, reparentTaskRecord.targetCloudPath) } + func testMoveFolderLocallyUpdatesDescendantCloudPaths() throws { + let rootItemMetadata = ItemMetadata(id: NSFileProviderItemIdentifier.rootContainerDatabaseValue, name: "Home", type: .folder, size: nil, parentID: NSFileProviderItemIdentifier.rootContainerDatabaseValue, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/"), isPlaceholderItem: false) + try metadataManagerMock.cacheMetadata(rootItemMetadata) + + let sourceParentID: Int64 = 2 + let movedFolderID: Int64 = 3 + let childFileID: Int64 = 4 + let targetParentID: Int64 = 5 + + // Initial tree: + // / + // |- A/ + // | |- B/ + // | |- C.txt + // |- Target/ + let sourceParent = ItemMetadata(id: sourceParentID, name: "A", type: .folder, size: nil, parentID: NSFileProviderItemIdentifier.rootContainerDatabaseValue, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/"), isPlaceholderItem: false) + let movedFolder = ItemMetadata(id: movedFolderID, name: "B", type: .folder, size: nil, parentID: sourceParentID, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/B/"), isPlaceholderItem: false) + let childFile = ItemMetadata(id: childFileID, name: "C.txt", type: .file, size: 100, parentID: movedFolderID, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/B/C.txt"), isPlaceholderItem: false) + let targetParent = ItemMetadata(id: targetParentID, name: "Target", type: .folder, size: nil, parentID: NSFileProviderItemIdentifier.rootContainerDatabaseValue, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/Target/"), isPlaceholderItem: false) + try metadataManagerMock.cacheMetadata([sourceParent, movedFolder, childFile, targetParent]) + + let movedFolderIdentifier = NSFileProviderItemIdentifier(domainIdentifier: .test, itemID: movedFolderID) + let targetParentIdentifier = NSFileProviderItemIdentifier(domainIdentifier: .test, itemID: targetParentID) + + // Move B from /A/B/ to /Target/B/. + // The important part is that descendants must follow that move as well in the database. + _ = try adapter.moveItemLocally(withIdentifier: movedFolderIdentifier, toParentItemWithIdentifier: targetParentIdentifier, newName: nil) + + // Sanity check for the folder row itself. + XCTAssertEqual(CloudPath("/Target/B/"), movedFolder.cloudPath) + XCTAssertEqual(targetParentID, movedFolder.parentID) + + // Regression check: + // If cloudPath is effectively hardcoded per row and only the moved folder row is updated, + // the child would incorrectly stay at /A/B/C.txt even though its parent is now /Target/B/. + // We expect the descendant path prefix to be rewritten to keep parentID and cloudPath in sync. + // Otherwise path-based metadata lookups, subtree queries, enumeration, deletion bookkeeping, + // and follow-up remote operations can use the stale location. + // This does not directly corrupt the local cached-file table because that is keyed by item id/local URL. + let updatedChild = try XCTUnwrap(metadataManagerMock.getCachedMetadata(for: childFileID)) + XCTAssertEqual(CloudPath("/Target/B/C.txt"), updatedChild.cloudPath) + XCTAssertEqual(movedFolderID, updatedChild.parentID) + XCTAssertNil(try metadataManagerMock.getCachedMetadata(for: CloudPath("/A/B/C.txt"))) + } + func testRenameItem() throws { let expectation = XCTestExpectation() From b1a5600f36594c66a25c3762092907a279735aa0 Mon Sep 17 00:00:00 2001 From: Tobias Hagemann Date: Thu, 14 May 2026 15:29:49 +0200 Subject: [PATCH 2/2] Rewrite descendant cloudPath on folder move and repair stale rows moveItemLocally only updated the moved folder's row, so every descendant's cloudPath went stale while its parentID chain stayed correct. Path-keyed lookups (getCachedMetadata(for: CloudPath), getAllCachedMetadata(inside:), checkLocalItemCollision, getItemIdentifier(for:)) then saw inconsistent state. Rewrite descendants recursively after the folder row is updated, using parentID to walk the subtree. A v5 repair migration fixes already-corrupted databases by walking the tree from root and rewriting any cloudPath whose canonical value disagrees with the stored value; unreachable rows and rows whose canonical slot is occupied are left alone. Refs #450. --- Cryptomator.xcodeproj/project.pbxproj | 4 + .../DB/DatabaseHelper.swift | 54 ++++ .../FileProviderAdapter.swift | 20 ++ .../DB/DatabaseHelperMigrationTests.swift | 261 ++++++++++++++++++ .../FileProviderAdapterMoveItemTests.swift | 37 +++ 5 files changed, 376 insertions(+) create mode 100644 CryptomatorFileProviderTests/DB/DatabaseHelperMigrationTests.swift diff --git a/Cryptomator.xcodeproj/project.pbxproj b/Cryptomator.xcodeproj/project.pbxproj index db7d42a72..747f98f9d 100644 --- a/Cryptomator.xcodeproj/project.pbxproj +++ b/Cryptomator.xcodeproj/project.pbxproj @@ -453,6 +453,7 @@ B3C397FE2EB10FC0001280AC /* ShareVaultViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3C397FC2EB10FC0001280AC /* ShareVaultViewController.swift */; }; B3C398002EB110F9001280AC /* ShareVaultViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3C397FF2EB110F9001280AC /* ShareVaultViewModel.swift */; }; B3D19A442CB937C700CD18A5 /* FileProviderCoordinatorError.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3D19A432CB937BF00CD18A5 /* FileProviderCoordinatorError.swift */; }; + CAB100002600000000000003 /* DatabaseHelperMigrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAB100002600000000000004 /* DatabaseHelperMigrationTests.swift */; }; FB6962AFC1C60F6728A7850E /* FileProviderAdapterRecoverUploadsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 153B84C203850D7414DE2A66 /* FileProviderAdapterRecoverUploadsTests.swift */; }; /* End PBXBuildFile section */ @@ -1089,6 +1090,7 @@ B3C397FC2EB10FC0001280AC /* ShareVaultViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareVaultViewController.swift; sourceTree = ""; }; B3C397FF2EB110F9001280AC /* ShareVaultViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareVaultViewModel.swift; sourceTree = ""; }; B3D19A432CB937BF00CD18A5 /* FileProviderCoordinatorError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileProviderCoordinatorError.swift; sourceTree = ""; }; + CAB100002600000000000004 /* DatabaseHelperMigrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatabaseHelperMigrationTests.swift; sourceTree = ""; }; D4BFCEFE82DA5BB518E9DA8B /* th */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = th; path = th.lproj/Intents.strings; sourceTree = ""; }; /* End PBXFileReference section */ @@ -1938,6 +1940,7 @@ isa = PBXGroup; children = ( 4A4A3863253F2B1900EE3828 /* CachedFileManagerTests.swift */, + CAB100002600000000000004 /* DatabaseHelperMigrationTests.swift */, 4ABC08D6250D1EB600E3CEDC /* DeletionTaskManagerTests.swift */, 4A231B83271EFC6100987492 /* DownloadTaskManagerTests.swift */, 4A49FABD271ECDE80069A0CC /* ItemEnumerationTaskManagerTests.swift */, @@ -2732,6 +2735,7 @@ 4A9C8E0127A0104E000063E4 /* EnumerationSignalingMock.swift in Sources */, 4AEECD2F279EA27300C6E2B5 /* FileProviderAdapterSetTagDataTests.swift in Sources */, 4AEECD3D279EB4B200C6E2B5 /* FileProviderAdapterProvidingMock.swift in Sources */, + CAB100002600000000000003 /* DatabaseHelperMigrationTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/CryptomatorFileProvider/DB/DatabaseHelper.swift b/CryptomatorFileProvider/DB/DatabaseHelper.swift index 0a182e01f..0f1353492 100644 --- a/CryptomatorFileProvider/DB/DatabaseHelper.swift +++ b/CryptomatorFileProvider/DB/DatabaseHelper.swift @@ -6,6 +6,7 @@ // Copyright © 2020 Skymatic GmbH. All rights reserved. // +import CocoaLumberjackSwift import CryptomatorCloudAccessCore import FileProvider import Foundation @@ -209,9 +210,62 @@ public struct DatabaseHelper: DatabaseHelping { ) """) } + migrator.registerMigration("v5", foreignKeyChecks: .immediate) { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } try migrator.migrate(dbWriter) } + /** + Repairs `cloudPath` values that became stale because earlier versions of `moveItemLocally` updated only the moved folder's row. + + Walks the tree breadth-first from the root via `parentID` and rewrites any `cloudPath` whose canonical value (derived from the parent's path plus the row's `name`) differs from the stored value. Rows that are not reachable from the root (orphans, disconnected cycles) are intentionally left untouched. + + If a row's canonical path is already occupied by another row (e.g. a duplicate created after the stale-descendant bug), the conflicting row is left at its stale path and the conflict is logged. When that happens, the BFS does not descend into the conflicted folder's children — descending would rewrite descendants to a `/canonical/…` prefix whose `/canonical` parent stayed stale, splitting the subtree. + + Also creates an index on `itemMetadata.parentID` so future `parentID`-based lookups — including the runtime descendant rewrite — do not require a table scan. + */ + static func repairCloudPathsMigration(_ db: Database) throws { + try db.execute(sql: "CREATE INDEX IF NOT EXISTS itemMetadata_parentID ON itemMetadata(parentID)") + + var queue: [(parentID: Int64, parentPath: CloudPath)] = [(NSFileProviderItemIdentifier.rootContainerDatabaseValue, CloudPath("/"))] + var head = 0 + var visitedCount = 1 + while head < queue.count { + let (parentID, parentPath) = queue[head] + head += 1 + let rows = try Row.fetchAll(db, sql: """ + SELECT id, name, type, cloudPath + FROM itemMetadata + WHERE parentID = ? AND id != ? + """, arguments: [parentID, NSFileProviderItemIdentifier.rootContainerDatabaseValue]) + rows: for row in rows { + let id: Int64 = row["id"] + let name: String = row["name"] + let itemType: CloudItemType = row["type"] + let storedCloudPath: CloudPath = row["cloudPath"] + visitedCount += 1 + let canonical = parentPath.appendingPathComponent(name) + if storedCloudPath != canonical { + do { + try db.execute(sql: "UPDATE itemMetadata SET cloudPath = ? WHERE id = ?", arguments: [canonical, id]) + } catch let error as DatabaseError where error.extendedResultCode == .SQLITE_CONSTRAINT_UNIQUE { + DDLogError("Repair migration: cloudPath \(canonical) already occupied; leaving id=\(id) at \(storedCloudPath)") + continue rows + } + } + if itemType == .folder { + queue.append((id, canonical)) + } + } + } + + let totalCount = try Int.fetchOne(db, sql: "SELECT COUNT(*) FROM itemMetadata") ?? 0 + if visitedCount < totalCount { + DDLogInfo("Repair migration: \(totalCount - visitedCount) row(s) not reachable from root, left untouched") + } + } + private static func openSharedDatabase(at databaseURL: URL, fileCoordinator: NSFileCoordinator) throws -> DatabasePool { var coordinatorError: NSError? var dbPool: DatabasePool? diff --git a/CryptomatorFileProvider/FileProviderAdapter.swift b/CryptomatorFileProvider/FileProviderAdapter.swift index 3bf92e0b9..4a3ab40e0 100644 --- a/CryptomatorFileProvider/FileProviderAdapter.swift +++ b/CryptomatorFileProvider/FileProviderAdapter.swift @@ -577,12 +577,32 @@ public class FileProviderAdapter: FileProviderAdapterType { itemMetadata.parentID = parentID itemMetadata.statusCode = .isUploading try itemMetadataManager.updateMetadata(itemMetadata) + if itemMetadata.type == .folder, let id = itemMetadata.id { + try rewriteDescendantCloudPaths(ofFolderID: id, newParentCloudPath: cloudPath) + } let localCachedFileInfo = try cachedFileManager.getLocalCachedFileInfo(for: itemMetadata) let item = FileProviderItem(metadata: itemMetadata, domainIdentifier: domainIdentifier, localCachedFileInfo: localCachedFileInfo) return MoveItemLocallyResult(item: item, reparentTaskRecord: taskRecord) } + private func rewriteDescendantCloudPaths(ofFolderID folderID: Int64, newParentCloudPath: CloudPath) throws { + var visited: Set = [folderID] + try rewriteDescendantCloudPaths(ofFolderID: folderID, newParentCloudPath: newParentCloudPath, visited: &visited) + } + + private func rewriteDescendantCloudPaths(ofFolderID folderID: Int64, newParentCloudPath: CloudPath, visited: inout Set) throws { + for child in try itemMetadataManager.getCachedMetadata(withParentID: folderID) { + guard let childID = child.id, visited.insert(childID).inserted else { continue } + let childCloudPath = newParentCloudPath.appendingPathComponent(child.name) + child.cloudPath = childCloudPath + try itemMetadataManager.updateMetadata(child) + if child.type == .folder { + try rewriteDescendantCloudPaths(ofFolderID: childID, newParentCloudPath: childCloudPath, visited: &visited) + } + } + } + func validateItemName(_ name: String) throws { do { try ItemNameValidator.validateName(name) diff --git a/CryptomatorFileProviderTests/DB/DatabaseHelperMigrationTests.swift b/CryptomatorFileProviderTests/DB/DatabaseHelperMigrationTests.swift new file mode 100644 index 000000000..b4c7a4b84 --- /dev/null +++ b/CryptomatorFileProviderTests/DB/DatabaseHelperMigrationTests.swift @@ -0,0 +1,261 @@ +// +// DatabaseHelperMigrationTests.swift +// CryptomatorFileProviderTests +// +// Created by Tobias Hagemann on 13.05.26. +// Copyright © 2026 Skymatic GmbH. All rights reserved. +// + +import CryptomatorCloudAccessCore +import GRDB +import XCTest +@testable import CryptomatorFileProvider + +/// Tests for `DatabaseHelper.repairCloudPathsMigration`. +/// +/// The harness opens a fresh in-memory `DatabaseQueue`, runs `DatabaseHelper.migrate(_:)` to install +/// the schema (including a no-op v5 pass on the empty DB), seeds stale or disconnected rows via raw SQL, +/// and then invokes `DatabaseHelper.repairCloudPathsMigration(_:)` directly against the seeded state. +/// +/// Seeded folder paths use the bare (`/A`) form to match what `CloudPath.appendingPathComponent` writes +/// during the repair, which makes the byte-level assertions deterministic. +class DatabaseHelperMigrationTests: XCTestCase { + var database: DatabaseWriter! + + override func setUpWithError() throws { + database = try DatabaseQueue() + try DatabaseHelper.migrate(database) + } + + func testRepairMigrationFixesStaleDescendant() throws { + // /A/ (id=2, no children) and /Target/ (id=3) live under root. + // B (id=4) is under /Target/ by parentID, but its stored cloudPath is stale at /A/B/. + // C.txt (id=5) is under B by parentID, with stale cloudPath /A/B/C.txt. + try database.write { db in + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'A', 'folder', NULL, 1, NULL, 'isUploaded', '/A', 0, 0), + (3, 'Target', 'folder', NULL, 1, NULL, 'isUploaded', '/Target', 0, 0), + (4, 'B', 'folder', NULL, 3, NULL, 'isUploaded', '/A/B', 0, 0), + (5, 'C.txt', 'file', 0, 4, NULL, 'isUploaded', '/A/B/C.txt', 0, 0) + """) + } + + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + + try assertCloudPath("/Target/B", forID: 4) + try assertCloudPath("/Target/B/C.txt", forID: 5) + } + + func testRepairMigrationFixesDeepStaleSubtree() throws { + // Three-level subtree under /Target/ with stale paths still rooted at /A/. + try database.write { db in + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'A', 'folder', NULL, 1, NULL, 'isUploaded', '/A', 0, 0), + (3, 'Target', 'folder', NULL, 1, NULL, 'isUploaded', '/Target', 0, 0), + (4, 'B', 'folder', NULL, 3, NULL, 'isUploaded', '/A/B', 0, 0), + (5, 'C', 'folder', NULL, 4, NULL, 'isUploaded', '/A/B/C', 0, 0), + (6, 'D.txt', 'file', 0, 5, NULL, 'isUploaded', '/A/B/C/D.txt', 0, 0) + """) + } + + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + + try assertCloudPath("/Target/B", forID: 4) + try assertCloudPath("/Target/B/C", forID: 5) + try assertCloudPath("/Target/B/C/D.txt", forID: 6) + } + + func testRepairMigrationLeavesCorrectRowsUntouched() throws { + try database.write { db in + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'A', 'folder', NULL, 1, NULL, 'isUploaded', '/A', 0, 0), + (3, 'B', 'folder', NULL, 2, NULL, 'isUploaded', '/A/B', 0, 0), + (4, 'C.txt', 'file', 0, 3, NULL, 'isUploaded', '/A/B/C.txt', 0, 0) + """) + } + let pathsBefore = try fetchCloudPathsByID() + + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + + let pathsAfter = try fetchCloudPathsByID() + XCTAssertEqual(pathsBefore, pathsAfter) + } + + func testRepairMigrationLeavesDisconnectedRowsUntouched() throws { + // Disconnected rows must bypass foreign-key checks during seeding: + // - id=4 has parentID=9999 (no such parent). + // - id=5 and id=6 form an X↔Y cycle that is not reachable from root. + // A separate reachable stale row (id=7 under /Target/) verifies the migration still rewrites what it can. + try database.write { db in + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'A', 'folder', NULL, 1, NULL, 'isUploaded', '/A', 0, 0), + (3, 'Target', 'folder', NULL, 1, NULL, 'isUploaded', '/Target', 0, 0), + (7, 'B', 'folder', NULL, 3, NULL, 'isUploaded', '/A/B', 0, 0) + """) + } + // Corrupt-state seeds bypass the FK constraint on itemMetadata.parentID. + // PRAGMA foreign_keys is a no-op inside a transaction, so the seeds run via writeWithoutTransaction. + // `defer` restores FK enforcement even if any of the seed inserts throws. + try database.writeWithoutTransaction { db in + try db.execute(sql: "PRAGMA foreign_keys = OFF") + defer { try? db.execute(sql: "PRAGMA foreign_keys = ON") } + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (4, 'Orphan', 'file', 0, 9999, NULL, 'isUploaded', '/orphan', 0, 0), + (5, 'X', 'folder', NULL, 6, NULL, 'isUploaded', '/X', 0, 0), + (6, 'Y', 'folder', NULL, 5, NULL, 'isUploaded', '/Y', 0, 0) + """) + } + + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + + try assertCloudPath("/orphan", forID: 4) + try assertCloudPath("/X", forID: 5) + try assertCloudPath("/Y", forID: 6) + try assertCloudPath("/Target/B", forID: 7) + } + + func testRepairMigrationCreatesParentIDIndex() throws { + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + let indexName = try database.read { db in + try String.fetchOne(db, sql: "SELECT name FROM sqlite_master WHERE type='index' AND name='itemMetadata_parentID'") + } + XCTAssertEqual("itemMetadata_parentID", indexName) + } + + func testRepairMigrationSkipsCanonicalPathConflict() throws { + // /A (id=2) and /Target (id=3) live under root. + // Row id=4 is parented to Target by parentID but its stored cloudPath is stale at /A/B. + // Row id=5 already occupies the canonical /Target/B slot — the migration must leave id=4 at its stale path and log. + // id=4 has a child id=6 at /A/B/C.txt; the migration must NOT descend into id=4's subtree after the skip, or the descendant + // would be rewritten to /Target/B/C.txt under a parent that stayed at /A/B (split subtree). + try database.write { db in + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'A', 'folder', NULL, 1, NULL, 'isUploaded', '/A', 0, 0), + (3, 'Target', 'folder', NULL, 1, NULL, 'isUploaded', '/Target', 0, 0), + (4, 'B', 'folder', NULL, 3, NULL, 'isUploaded', '/A/B', 0, 0), + (5, 'B', 'folder', NULL, 3, NULL, 'isUploaded', '/Target/B', 0, 0), + (6, 'C.txt', 'file', 0, 4, NULL, 'isUploaded', '/A/B/C.txt', 0, 0) + """) + } + + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + + try assertCloudPath("/A/B", forID: 4) + try assertCloudPath("/Target/B", forID: 5) + try assertCloudPath("/A/B/C.txt", forID: 6) + } + + func testRepairMigrationFixesBranchingSubtree() throws { + // Branching tree exercises that the BFS index pointer advances across multiple + // enqueued sibling folders before descending into either subtree. + try database.write { db in + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'A', 'folder', NULL, 1, NULL, 'isUploaded', '/A', 0, 0), + (3, 'Target', 'folder', NULL, 1, NULL, 'isUploaded', '/Target', 0, 0), + (4, 'B', 'folder', NULL, 3, NULL, 'isUploaded', '/A/B', 0, 0), + (5, 'L', 'folder', NULL, 4, NULL, 'isUploaded', '/A/B/L', 0, 0), + (6, 'R', 'folder', NULL, 4, NULL, 'isUploaded', '/A/B/R', 0, 0), + (7, 'L.txt', 'file', 0, 5, NULL, 'isUploaded', '/A/B/L/L.txt', 0, 0), + (8, 'R.txt', 'file', 0, 6, NULL, 'isUploaded', '/A/B/R/R.txt', 0, 0) + """) + } + + try database.write { db in + try DatabaseHelper.repairCloudPathsMigration(db) + } + + try assertCloudPath("/Target/B", forID: 4) + try assertCloudPath("/Target/B/L", forID: 5) + try assertCloudPath("/Target/B/R", forID: 6) + try assertCloudPath("/Target/B/L/L.txt", forID: 7) + try assertCloudPath("/Target/B/R/R.txt", forID: 8) + } + + func testRepairMigrationRunsAsRegisteredV5() throws { + // Simulate a pre-v5 install: clear v5's GRDB marker and drop the index it created, seed a stale row, + // then call DatabaseHelper.migrate(_:) so the migrator re-applies v5 end-to-end rather than via a direct helper call. + try database.write { db in + try db.execute(sql: "DELETE FROM grdb_migrations WHERE identifier = 'v5'") + try db.execute(sql: "DROP INDEX IF EXISTS itemMetadata_parentID") + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'Target', 'folder', NULL, 1, NULL, 'isUploaded', '/Target', 0, 0), + (3, 'B', 'folder', NULL, 2, NULL, 'isUploaded', '/A/B', 0, 0) + """) + } + + try DatabaseHelper.migrate(database) + + try assertCloudPath("/Target/B", forID: 3) + let indexName = try database.read { db in + try String.fetchOne(db, sql: "SELECT name FROM sqlite_master WHERE type='index' AND name='itemMetadata_parentID'") + } + XCTAssertEqual("itemMetadata_parentID", indexName) + } + + func testRepairMigrationRunsAsRegisteredV5WithPreExistingOrphan() throws { + // Pre-existing orphan rows would fail the default deferred-FK check at COMMIT time. + // v5 registers with foreignKeyChecks: .immediate so the FK sweep only checks rows the migration modifies. + // This test proves the registered migration tolerates an orphan (`parentID = 9999`) on its way through the migrator. + try database.write { db in + try db.execute(sql: "DELETE FROM grdb_migrations WHERE identifier = 'v5'") + try db.execute(sql: "DROP INDEX IF EXISTS itemMetadata_parentID") + } + try database.writeWithoutTransaction { db in + try db.execute(sql: "PRAGMA foreign_keys = OFF") + defer { try? db.execute(sql: "PRAGMA foreign_keys = ON") } + try db.execute(sql: """ + INSERT INTO itemMetadata (id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated) + VALUES + (2, 'Orphan', 'file', 0, 9999, NULL, 'isUploaded', '/orphan', 0, 0) + """) + } + + try DatabaseHelper.migrate(database) + + try assertCloudPath("/orphan", forID: 2) + } + + private func assertCloudPath(_ expected: String, forID id: Int64, file: StaticString = #file, line: UInt = #line) throws { + let actual = try database.read { db in + try String.fetchOne(db, sql: "SELECT cloudPath FROM itemMetadata WHERE id = ?", arguments: [id]) + } + XCTAssertEqual(expected, actual, "Unexpected cloudPath for id=\(id)", file: file, line: line) + } + + private func fetchCloudPathsByID() throws -> [Int64: String] { + try database.read { db in + let rows = try Row.fetchAll(db, sql: "SELECT id, cloudPath FROM itemMetadata") + return rows.reduce(into: [Int64: String]()) { acc, row in + acc[row["id"]] = row["cloudPath"] + } + } + } +} diff --git a/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift b/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift index 642ec37fe..12d4938e9 100644 --- a/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift +++ b/CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterMoveItemTests.swift @@ -173,6 +173,43 @@ class FileProviderAdapterMoveItemTests: FileProviderAdapterTestCase { XCTAssertNil(try metadataManagerMock.getCachedMetadata(for: CloudPath("/A/B/C.txt"))) } + func testMoveFolderLocallyUpdatesDeepDescendantCloudPaths() throws { + let rootItemMetadata = ItemMetadata(id: NSFileProviderItemIdentifier.rootContainerDatabaseValue, name: "Home", type: .folder, size: nil, parentID: NSFileProviderItemIdentifier.rootContainerDatabaseValue, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/"), isPlaceholderItem: false) + try metadataManagerMock.cacheMetadata(rootItemMetadata) + + let sourceParentID: Int64 = 2 + let movedFolderID: Int64 = 3 + let middleFolderID: Int64 = 4 + let deepFileID: Int64 = 5 + let targetParentID: Int64 = 6 + + // Initial tree: + // / + // |- A/ + // | |- B/ + // | |- C/ + // | |- D.txt + // |- Target/ + let sourceParent = ItemMetadata(id: sourceParentID, name: "A", type: .folder, size: nil, parentID: NSFileProviderItemIdentifier.rootContainerDatabaseValue, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/"), isPlaceholderItem: false) + let movedFolder = ItemMetadata(id: movedFolderID, name: "B", type: .folder, size: nil, parentID: sourceParentID, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/B/"), isPlaceholderItem: false) + let middleFolder = ItemMetadata(id: middleFolderID, name: "C", type: .folder, size: nil, parentID: movedFolderID, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/B/C/"), isPlaceholderItem: false) + let deepFile = ItemMetadata(id: deepFileID, name: "D.txt", type: .file, size: 100, parentID: middleFolderID, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/A/B/C/D.txt"), isPlaceholderItem: false) + let targetParent = ItemMetadata(id: targetParentID, name: "Target", type: .folder, size: nil, parentID: NSFileProviderItemIdentifier.rootContainerDatabaseValue, lastModifiedDate: nil, statusCode: .isUploaded, cloudPath: CloudPath("/Target/"), isPlaceholderItem: false) + try metadataManagerMock.cacheMetadata([sourceParent, movedFolder, middleFolder, deepFile, targetParent]) + + let movedFolderIdentifier = NSFileProviderItemIdentifier(domainIdentifier: .test, itemID: movedFolderID) + let targetParentIdentifier = NSFileProviderItemIdentifier(domainIdentifier: .test, itemID: targetParentID) + _ = try adapter.moveItemLocally(withIdentifier: movedFolderIdentifier, toParentItemWithIdentifier: targetParentIdentifier, newName: nil) + + XCTAssertEqual(CloudPath("/Target/B/"), movedFolder.cloudPath) + let updatedMiddle = try XCTUnwrap(metadataManagerMock.getCachedMetadata(for: middleFolderID)) + XCTAssertEqual(CloudPath("/Target/B/C/"), updatedMiddle.cloudPath) + XCTAssertEqual(movedFolderID, updatedMiddle.parentID) + let updatedDeepFile = try XCTUnwrap(metadataManagerMock.getCachedMetadata(for: deepFileID)) + XCTAssertEqual(CloudPath("/Target/B/C/D.txt"), updatedDeepFile.cloudPath) + XCTAssertEqual(middleFolderID, updatedDeepFile.parentID) + } + func testRenameItem() throws { let expectation = XCTestExpectation()