Skip to content

Derive cloudPath from parentID chain instead of denormalized column #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tobihagemann wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Derive cloudPath from parentID chain instead of denormalized column #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2660fb9
Derive cloudPath from parentID chain instead of denormalized column
tobihagemann May 17, 2026
725c59d
Add regression tests for descendant cloudPath after folder move
tobihagemann May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions CryptomatorFileProvider/CloudTask/CloudTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import Foundation

protocol CloudTask {
var itemMetadata: ItemMetadata { get }
}

extension CloudTask {
var cloudPath: CloudPath {
return itemMetadata.cloudPath
}
/// Snapshot captured at task construction; survives concurrent local renames of the same row.
var cloudPath: CloudPath { get }
}
10 changes: 5 additions & 5 deletions CryptomatorFileProvider/CloudTask/DeletionTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
// Copyright © 2020 Skymatic GmbH. All rights reserved.
//

import GRDB
import CryptomatorCloudAccessCore

struct DeletionTask: CloudTask, FetchableRecord, Decodable {
struct DeletionTask: CloudTask {
let taskRecord: DeletionTaskRecord
let itemMetadata: ItemMetadata
let cloudPath: CloudPath

enum CodingKeys: String, CodingKey {
case taskRecord = "deletionTask"
case itemMetadata
func with(cloudPath: CloudPath) -> DeletionTask {
return DeletionTask(taskRecord: taskRecord, itemMetadata: itemMetadata, cloudPath: cloudPath)
}
}
8 changes: 4 additions & 4 deletions CryptomatorFileProvider/CloudTask/DownloadTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
// Copyright © 2021 Skymatic GmbH. All rights reserved.
//

import CryptomatorCloudAccessCore
import Foundation
import GRDB

struct DownloadTask: CloudTask {
let taskRecord: DownloadTaskRecord
let itemMetadata: ItemMetadata
let cloudPath: CloudPath
let onURLSessionTaskCreation: URLSessionTaskCreationClosure?

enum CodingKeys: String, CodingKey {
case taskRecord = "downloadTask"
case itemMetadata
func with(cloudPath: CloudPath) -> DownloadTask {
return DownloadTask(taskRecord: taskRecord, itemMetadata: itemMetadata, cloudPath: cloudPath, onURLSessionTaskCreation: onURLSessionTaskCreation)
}
}

Expand Down
7 changes: 6 additions & 1 deletion CryptomatorFileProvider/CloudTask/FolderCreationTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
// Copyright © 2021 Skymatic GmbH. All rights reserved.
//

import Foundation
import CryptomatorCloudAccessCore

struct FolderCreationTask: CloudTask {
let itemMetadata: ItemMetadata
let cloudPath: CloudPath

func with(cloudPath: CloudPath) -> FolderCreationTask {
return FolderCreationTask(itemMetadata: itemMetadata, cloudPath: cloudPath)
}
}
9 changes: 4 additions & 5 deletions CryptomatorFileProvider/CloudTask/ItemEnumerationTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
// Copyright © 2021 Skymatic GmbH. All rights reserved.
//

import Foundation
import GRDB
import CryptomatorCloudAccessCore

struct ItemEnumerationTask: CloudTask {
let taskRecord: ItemEnumerationTaskRecord
let itemMetadata: ItemMetadata
let cloudPath: CloudPath

enum CodingKeys: String, CodingKey {
case taskRecord = "itemEnumerationTask"
case itemMetadata
func with(cloudPath: CloudPath) -> ItemEnumerationTask {
return ItemEnumerationTask(taskRecord: taskRecord, itemMetadata: itemMetadata, cloudPath: cloudPath)
}
}
10 changes: 5 additions & 5 deletions CryptomatorFileProvider/CloudTask/ReparentTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
// Copyright © 2020 Skymatic GmbH. All rights reserved.
//

import GRDB
import CryptomatorCloudAccessCore

struct ReparentTask: CloudTask, FetchableRecord, Decodable {
struct ReparentTask: CloudTask {
let taskRecord: ReparentTaskRecord
let itemMetadata: ItemMetadata
let cloudPath: CloudPath

enum CodingKeys: String, CodingKey {
case taskRecord = "reparentTask"
case itemMetadata
func with(cloudPath: CloudPath, taskRecord: ReparentTaskRecord) -> ReparentTask {
return ReparentTask(taskRecord: taskRecord, itemMetadata: itemMetadata, cloudPath: cloudPath)
}
}
7 changes: 6 additions & 1 deletion CryptomatorFileProvider/CloudTask/UploadTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@
// Copyright © 2020 Skymatic GmbH. All rights reserved.
//

import CryptomatorCloudAccessCore
import Foundation
import GRDB

struct UploadTask: CloudTask {
let taskRecord: UploadTaskRecord
let itemMetadata: ItemMetadata
let cloudPath: CloudPath
let onURLSessionTaskCreation: URLSessionTaskCreationClosure?

func with(cloudPath: CloudPath) -> UploadTask {
return UploadTask(taskRecord: taskRecord, itemMetadata: itemMetadata, cloudPath: cloudPath, onURLSessionTaskCreation: onURLSessionTaskCreation)
}
}
26 changes: 26 additions & 0 deletions CryptomatorFileProvider/DB/DatabaseHelper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,32 @@ public struct DatabaseHelper: DatabaseHelping {
)
""")
}
migrator.registerMigration("v5", foreignKeyChecks: .deferred) { db in
// SQLite refuses DROP COLUMN on UNIQUE-constrained columns, so we must rebuild to drop `cloudPath`.
try db.execute(sql: """
CREATE TABLE itemMetadata_tmp (
id INTEGER PRIMARY KEY AUTOINCREMENT,
name TEXT NOT NULL,
type TEXT NOT NULL,
size INTEGER,
parentID INTEGER REFERENCES itemMetadata(id) ON DELETE CASCADE,
lastModifiedDate DATE,
statusCode TEXT NOT NULL,
isPlaceholderItem BOOLEAN NOT NULL DEFAULT 0,
isMaybeOutdated BOOLEAN NOT NULL DEFAULT 0,
favoriteRank INTEGER,
tagData BLOB,
lastEnumeratedAt DATE
)
""")
try db.execute(sql: """
INSERT INTO itemMetadata_tmp (id, name, type, size, parentID, lastModifiedDate, statusCode, isPlaceholderItem, isMaybeOutdated, favoriteRank, tagData, lastEnumeratedAt)
SELECT id, name, type, size, parentID, lastModifiedDate, statusCode, isPlaceholderItem, isMaybeOutdated, favoriteRank, tagData, lastEnumeratedAt FROM itemMetadata
""")
try db.execute(sql: "DROP TABLE itemMetadata")
try db.execute(sql: "ALTER TABLE itemMetadata_tmp RENAME TO itemMetadata")
try db.execute(sql: "CREATE INDEX itemMetadata_parentID_idx ON itemMetadata(parentID)")
}
Comment on lines +212 to +237
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Preserve or eliminate preexisting case-only sibling duplicates during v5.

This migration drops cloudPath, but it does not reconcile rows that were previously legal under the old unique cloudPath constraint yet now collide under the new (parentID, lowercased(name)) lookup semantics. After upgrade, childOfFolder/cacheMetadata can resolve either sibling nondeterministically and update the wrong row. Please add a repair step here, or fail the migration when such collisions exist, before relying on case-insensitive descent.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CryptomatorFileProvider/DB/DatabaseHelper.swift` around lines 212 - 237,
Before rebuilding itemMetadata in migrator.registerMigration("v5"), detect and
handle case-only sibling collisions by querying itemMetadata grouped by parentID
and lower(name) (e.g., SELECT parentID, lower(name) AS lname, GROUP_CONCAT(id)
ids, COUNT(*) cnt FROM itemMetadata GROUP BY parentID, lname HAVING cnt > 1); if
any rows are returned either abort the migration with a clear MigrationError
including the offending ids (so upgrade can be fixed manually) or
deterministically resolve them (choose one row to keep, e.g., MIN(id) or newest
lastEnumeratedAt, and delete the other ids) before proceeding to
CREATE/INSERT/DROP/RENAME; implement this check/repair inside the same
migrator.registerMigration("v5") block referencing itemMetadata and cloudPath so
childOfFolder/cacheMetadata will not later pick the wrong row.

try migrator.migrate(dbWriter)
}

Expand Down
14 changes: 9 additions & 5 deletions CryptomatorFileProvider/DB/DeletionTaskDBManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
// Copyright © 2020 Skymatic GmbH. All rights reserved.
//

import CryptomatorCloudAccessCore
import Foundation
import GRDB

protocol DeletionTaskManager {
func createTaskRecord(for item: ItemMetadata) throws -> DeletionTaskRecord
func createTaskRecord(for item: ItemMetadata, cloudPath: CloudPath) throws -> DeletionTaskRecord
func getTaskRecord(for id: Int64) throws -> DeletionTaskRecord
func removeTaskRecord(_ task: DeletionTaskRecord) throws
func getTaskRecordsForItemsWhichWere(in parentID: Int64) throws -> [DeletionTaskRecord]
Expand All @@ -27,9 +28,12 @@ class DeletionTaskDBManager: DeletionTaskManager {
}
}

func createTaskRecord(for item: ItemMetadata) throws -> DeletionTaskRecord {
try database.write { db in
let task = DeletionTaskRecord(correspondingItem: item.id!, cloudPath: item.cloudPath, parentID: item.parentID, itemType: item.type)
func createTaskRecord(for item: ItemMetadata, cloudPath: CloudPath) throws -> DeletionTaskRecord {
guard let id = item.id else {
throw DBManagerError.nonSavedItemMetadata
}
return try database.write { db in
let task = DeletionTaskRecord(correspondingItem: id, cloudPath: cloudPath, parentID: item.parentID, itemType: item.type)
try task.save(db)
return task
}
Expand Down Expand Up @@ -63,7 +67,7 @@ class DeletionTaskDBManager: DeletionTaskManager {
guard let itemMetadata = try taskRecord.itemMetadata.fetchOne(db) else {
throw DBManagerError.missingItemMetadata
}
return DeletionTask(taskRecord: taskRecord, itemMetadata: itemMetadata)
return DeletionTask(taskRecord: taskRecord, itemMetadata: itemMetadata, cloudPath: taskRecord.cloudPath)
}
}
}
17 changes: 12 additions & 5 deletions CryptomatorFileProvider/DB/DownloadTaskDBManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// Copyright © 2021 Skymatic GmbH. All rights reserved.
//

import CryptomatorCloudAccessCore
import Foundation
import GRDB

Expand All @@ -16,19 +17,25 @@ protocol DownloadTaskManager {

class DownloadTaskDBManager: DownloadTaskManager {
private let database: DatabaseWriter
private let itemMetadataManager: ItemMetadataManager

init(database: DatabaseWriter) throws {
init(database: DatabaseWriter, itemMetadataManager: ItemMetadataManager) throws {
self.database = database
self.itemMetadataManager = itemMetadataManager
_ = try database.write { db in
try ItemEnumerationTaskRecord.deleteAll(db)
try DownloadTaskRecord.deleteAll(db)
}
}

func createTask(for item: ItemMetadata, replaceExisting: Bool, localURL: URL, onURLSessionTaskCreation: URLSessionTaskCreationClosure?) throws -> DownloadTask {
try database.write { db in
let taskRecord = DownloadTaskRecord(correspondingItem: item.id!, replaceExisting: replaceExisting, localURL: localURL)
guard let id = item.id else {
throw DBManagerError.nonSavedItemMetadata
}
let cloudPath = try itemMetadataManager.getCloudPath(for: id)
return try database.write { db in
let taskRecord = DownloadTaskRecord(correspondingItem: id, replaceExisting: replaceExisting, localURL: localURL)
try taskRecord.save(db)
return DownloadTask(taskRecord: taskRecord, itemMetadata: item, onURLSessionTaskCreation: onURLSessionTaskCreation)
return DownloadTask(taskRecord: taskRecord, itemMetadata: item, cloudPath: cloudPath, onURLSessionTaskCreation: onURLSessionTaskCreation)
}
}

Expand Down
15 changes: 11 additions & 4 deletions CryptomatorFileProvider/DB/ItemEnumerationTaskDBManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// Copyright © 2021 Skymatic GmbH. All rights reserved.
//

import CryptomatorCloudAccessCore
import Foundation
import GRDB

Expand All @@ -16,19 +17,25 @@ protocol ItemEnumerationTaskManager {

class ItemEnumerationTaskDBManager: ItemEnumerationTaskManager {
private let database: DatabaseWriter
private let itemMetadataManager: ItemMetadataManager

init(database: DatabaseWriter) throws {
init(database: DatabaseWriter, itemMetadataManager: ItemMetadataManager) throws {
self.database = database
self.itemMetadataManager = itemMetadataManager
_ = try database.write { db in
try ItemEnumerationTaskRecord.deleteAll(db)
}
}

func createTask(for item: ItemMetadata, pageToken: String?) throws -> ItemEnumerationTask {
try database.write { db in
let taskRecord = ItemEnumerationTaskRecord(correspondingItem: item.id!, pageToken: pageToken)
guard let id = item.id else {
throw DBManagerError.nonSavedItemMetadata
}
let cloudPath = try itemMetadataManager.getCloudPath(for: id)
return try database.write { db in
let taskRecord = ItemEnumerationTaskRecord(correspondingItem: id, pageToken: pageToken)
try taskRecord.save(db)
return ItemEnumerationTask(taskRecord: taskRecord, itemMetadata: item)
return ItemEnumerationTask(taskRecord: taskRecord, itemMetadata: item, cloudPath: cloudPath)
}
}

Expand Down
12 changes: 4 additions & 8 deletions CryptomatorFileProvider/DB/ItemMetadata.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public class ItemMetadata: Record, Codable {
var parentID: Int64
var lastModifiedDate: Date?
var statusCode: ItemStatus
var cloudPath: CloudPath
var isPlaceholderItem: Bool
var isMaybeOutdated: Bool
var favoriteRank: Int64?
Expand All @@ -41,7 +40,6 @@ public class ItemMetadata: Record, Codable {
self.parentID = row[Columns.parentID]
self.lastModifiedDate = row[Columns.lastModifiedDate]
self.statusCode = row[Columns.statusCode]
self.cloudPath = row[Columns.cloudPath]
self.isPlaceholderItem = row[Columns.isPlaceholderItem]
self.isMaybeOutdated = row[Columns.isMaybeOutdated]
self.favoriteRank = row[Columns.favoriteRank]
Expand All @@ -51,18 +49,17 @@ public class ItemMetadata: Record, Codable {
}

convenience init(item: CloudItemMetadata, withParentID parentID: Int64, isPlaceholderItem: Bool = false) {
self.init(name: item.name, type: item.itemType, size: item.size, parentID: parentID, lastModifiedDate: item.lastModifiedDate, statusCode: .isUploaded, cloudPath: item.cloudPath, isPlaceholderItem: isPlaceholderItem)
self.init(name: item.name, type: item.itemType, size: item.size, parentID: parentID, lastModifiedDate: item.lastModifiedDate, statusCode: .isUploaded, isPlaceholderItem: isPlaceholderItem)
}

init(id: Int64? = nil, name: String, type: CloudItemType, size: Int?, parentID: Int64, lastModifiedDate: Date?, statusCode: ItemStatus, cloudPath: CloudPath, isPlaceholderItem: Bool, isCandidateForCacheCleanup: Bool = false, favoriteRank: Int64? = nil, tagData: Data? = nil, lastEnumeratedAt: Date? = nil) {
init(id: Int64? = nil, name: String, type: CloudItemType, size: Int?, parentID: Int64, lastModifiedDate: Date?, statusCode: ItemStatus, isPlaceholderItem: Bool, isCandidateForCacheCleanup: Bool = false, favoriteRank: Int64? = nil, tagData: Data? = nil, lastEnumeratedAt: Date? = nil) {
self.id = id
self.name = name
self.type = type
self.size = size
self.parentID = parentID
self.lastModifiedDate = lastModifiedDate
self.statusCode = statusCode
self.cloudPath = cloudPath
self.isPlaceholderItem = isPlaceholderItem
self.isMaybeOutdated = isCandidateForCacheCleanup
self.favoriteRank = favoriteRank
Expand All @@ -83,7 +80,6 @@ public class ItemMetadata: Record, Codable {
container[Columns.parentID] = parentID
container[Columns.lastModifiedDate] = lastModifiedDate
container[Columns.statusCode] = statusCode
container[Columns.cloudPath] = cloudPath
container[Columns.isPlaceholderItem] = isPlaceholderItem
container[Columns.isMaybeOutdated] = isMaybeOutdated
container[Columns.favoriteRank] = favoriteRank
Expand All @@ -92,7 +88,7 @@ public class ItemMetadata: Record, Codable {
}

enum Columns: String, ColumnExpression {
case id, name, type, size, parentID, lastModifiedDate, statusCode, cloudPath, isPlaceholderItem, isMaybeOutdated, favoriteRank, tagData, lastEnumeratedAt
case id, name, type, size, parentID, lastModifiedDate, statusCode, isPlaceholderItem, isMaybeOutdated, favoriteRank, tagData, lastEnumeratedAt
}

static func filterWorkingSet() -> QueryInterfaceRequest<ItemMetadata> {
Expand All @@ -103,6 +99,6 @@ public class ItemMetadata: Record, Codable {
extension ItemStatus: DatabaseValueConvertible {}
extension ItemMetadata: Equatable {
public static func == (lhs: ItemMetadata, rhs: ItemMetadata) -> Bool {
lhs.id == rhs.id && lhs.name == rhs.name && lhs.type == rhs.type && lhs.size == rhs.size && lhs.parentID == rhs.parentID && lhs.lastModifiedDate == rhs.lastModifiedDate && lhs.statusCode == rhs.statusCode && lhs.cloudPath == rhs.cloudPath && lhs.isPlaceholderItem == rhs.isPlaceholderItem && lhs.isMaybeOutdated == rhs.isMaybeOutdated && lhs.favoriteRank == rhs.favoriteRank && lhs.tagData == rhs.tagData && lhs.lastEnumeratedAt == rhs.lastEnumeratedAt
lhs.id == rhs.id && lhs.name == rhs.name && lhs.type == rhs.type && lhs.size == rhs.size && lhs.parentID == rhs.parentID && lhs.lastModifiedDate == rhs.lastModifiedDate && lhs.statusCode == rhs.statusCode && lhs.isPlaceholderItem == rhs.isPlaceholderItem && lhs.isMaybeOutdated == rhs.isMaybeOutdated && lhs.favoriteRank == rhs.favoriteRank && lhs.tagData == rhs.tagData && lhs.lastEnumeratedAt == rhs.lastEnumeratedAt
}
}
Loading
Loading