fix(sidebar): write favorite tables once per change and give each its own iCloud record

CHANGELOG.md

@@ -58,6 +58,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `EXPLAIN ANALYSE` treated as a plain `EXPLAIN` that does not run its statement.
 - Safe Mode level lost when importing a connection file from Mac to iOS or from iOS to Mac.
 - Undo and Redo in a tab with unsaved edits replaying another tab's changes against the wrong rows.
+- App pausing while renaming or dropping a database or schema that holds many favorite tables.
+- iCloud sync mixing up two favorite tables whose names contain a vertical bar.
+- iCloud sync sending both a save and a deletion for an item unstarred and starred again, or renamed back, before it ran.
+- Favorite table starred again while its removal was syncing to iCloud disappearing when the sync finished.
+- Table favorites, saved queries and column layouts of **Local only** connections syncing to iCloud.
+- Database favorites and column layouts removed on another Mac staying on this one.
 - MongoDB, Elasticsearch, Typesense and SurrealDB saving a long array or object shortened for display as the cut text.
 - MongoDB refusing text like `[DRAFT] Chapter one...` as a value shortened for display.
 - Clipboard URL banner turning `sslmode=verify-full` or `verify-ca` into Required and ignoring `sslmode=disable`.

Packages/TableProCore/Sources/TableProSyncTransport/SyncMetadataStorage.swift

@@ -2,13 +2,15 @@ import CloudKit
 import Foundation
 import os
 
-public struct Tombstone: Codable, Sendable {
+public struct Tombstone: Codable, Equatable, Sendable {
     public let id: String
     public let deletedAt: Date
+    public let owner: UUID?
 
-    public init(id: String, deletedAt: Date = Date()) {
+    public init(id: String, deletedAt: Date = Date(), owner: UUID? = nil) {
         self.id = id
         self.deletedAt = deletedAt
+        self.owner = owner
     }
 }
 
@@ -108,11 +110,13 @@ public final class SyncMetadataStorage: @unchecked Sendable {
         addTombstones([id], type: type)
     }
 
-    public func addTombstones(_ ids: [String], type: SyncRecordType) {
-        guard !ids.isEmpty else { return }
-        var current = tombstones(for: type)
-        current.append(contentsOf: ids.map { Tombstone(id: $0) })
-        saveTombstones(current, for: type)
+    public func addTombstones(_ ids: [String], type: SyncRecordType, owner: UUID? = nil) {
+        addTombstones(ids.map { Tombstone(id: $0, owner: owner) }, type: type)
+    }
+
+    public func addTombstones(_ added: [Tombstone], type: SyncRecordType) {
+        guard !added.isEmpty else { return }
+        saveTombstones(tombstones(for: type) + added, for: type)
     }
 
     /// Records the record types the running build reads and reports whether that set grew. A set
@@ -147,17 +151,50 @@ public final class SyncMetadataStorage: @unchecked Sendable {
         userDefaults.removeObject(forKey: tombstoneKey(type))
     }
 
-    public func pruneTombstones(olderThan days: Int) {
+    public func pruneTombstones(
+        olderThan days: Int,
+        where isPushable: (SyncRecordType, Tombstone) -> Bool
+    ) {
         let cutoff = Calendar.current.date(byAdding: .day, value: -days, to: Date()) ?? Date()
+        removeTombstones { type, tombstone in
+            tombstone.deletedAt < cutoff && isPushable(type, tombstone)
+        }
+    }
+
+    public func removeTombstones(where shouldRemove: (SyncRecordType, Tombstone) -> Bool) {
         for type in SyncRecordType.allCases {
             var current = tombstones(for: type)
             let before = current.count
-            current.removeAll { $0.deletedAt < cutoff }
+            current.removeAll { shouldRemove(type, $0) }
             guard current.count != before else { continue }
             saveTombstones(current, for: type)
         }
     }
 
+    // MARK: - Owners Kept Off Sync
+
+    public func ownersKeptOffSync() -> Set<UUID> {
+        Set((userDefaults.stringArray(forKey: key("ownersKeptOffSync")) ?? []).compactMap(UUID.init(uuidString:)))
+    }
+
+    public func keepOffSync(owners: Set<UUID>) {
+        guard !owners.isEmpty else { return }
+        saveOwnersKeptOffSync(ownersKeptOffSync().union(owners))
+    }
+
+    public func releaseOwnersKeptOffSync(_ owners: Set<UUID>) {
+        guard !owners.isEmpty else { return }
+        saveOwnersKeptOffSync(ownersKeptOffSync().subtracting(owners))
+    }
+
+    private func saveOwnersKeptOffSync(_ owners: Set<UUID>) {
+        guard !owners.isEmpty else {
+            userDefaults.removeObject(forKey: key("ownersKeptOffSync"))
+            return
+        }
+        userDefaults.set(owners.map(\.uuidString).sorted(), forKey: key("ownersKeptOffSync"))
+    }
+
     // MARK: - Last Sync Date
 
     public var lastSyncDate: Date? {

Packages/TableProCore/Tests/TableProSyncTests/SyncMetadataStorageTests.swift

@@ -86,11 +86,82 @@ struct SyncMetadataStorageTests {
         defaults.set(data, forKey: "com.TablePro.sync.tombstones.\(SyncRecordType.connection.rawValue)")
 
         let storage = SyncMetadataStorage(userDefaults: defaults)
-        storage.pruneTombstones(olderThan: 30)
+        storage.pruneTombstones(olderThan: 30) { _, _ in true }
 
         #expect(storage.tombstones(for: .connection).map(\.id) == ["fresh"])
     }
 
+    @Test("Pruning keeps an old tombstone that cannot be pushed yet")
+    func pruningKeepsUnpushableTombstones() throws {
+        let defaults = UserDefaults(suiteName: "com.TablePro.tests.\(UUID().uuidString)") ?? .standard
+        let heldOwner = UUID()
+        let fortyDaysAgo = Date(timeIntervalSinceNow: -60 * 60 * 24 * 40)
+        let held = Tombstone(id: "held", deletedAt: fortyDaysAgo, owner: heldOwner)
+        let released = Tombstone(id: "released", deletedAt: fortyDaysAgo, owner: UUID())
+        let data = try JSONEncoder().encode([held, released])
+        defaults.set(data, forKey: "com.TablePro.sync.tombstones.\(SyncRecordType.tableFavorite.rawValue)")
+
+        let storage = SyncMetadataStorage(userDefaults: defaults)
+        storage.pruneTombstones(olderThan: 30) { _, tombstone in tombstone.owner != heldOwner }
+
+        #expect(storage.tombstones(for: .tableFavorite).map(\.id) == ["held"])
+    }
+
+    @Test("A tombstone keeps the owner it was recorded with")
+    func tombstoneOwnerRoundTrips() {
+        let storage = makeStorage()
+        let owner = UUID()
+        storage.addTombstones(["a", "b"], type: .tableFavorite, owner: owner)
+        storage.addTombstone("c", type: .tableFavorite)
+
+        #expect(storage.tombstones(for: .tableFavorite).map(\.owner) == [owner, owner, nil])
+    }
+
+    @Test("A tombstone written before owners existed still decodes, with no owner")
+    func legacyTombstoneDecodesWithoutOwner() throws {
+        let defaults = UserDefaults(suiteName: "com.TablePro.tests.\(UUID().uuidString)") ?? .standard
+        let legacy = Data(#"[{"id":"a","deletedAt":780000000}]"#.utf8)
+        defaults.set(legacy, forKey: "com.TablePro.sync.tombstones.\(SyncRecordType.favoriteDatabase.rawValue)")
+
+        let tombstones = SyncMetadataStorage(userDefaults: defaults).tombstones(for: .favoriteDatabase)
+
+        #expect(tombstones.map(\.id) == ["a"])
+        #expect(tombstones.map(\.owner) == [nil])
+    }
+
+    @Test("Owners kept off sync survive a new storage instance and can be released one by one")
+    func ownersKeptOffSyncPersist() {
+        let defaults = UserDefaults(suiteName: "com.TablePro.tests.\(UUID().uuidString)") ?? .standard
+        let first = UUID()
+        let second = UUID()
+        SyncMetadataStorage(userDefaults: defaults).keepOffSync(owners: [first, second])
+
+        let storage = SyncMetadataStorage(userDefaults: defaults)
+        #expect(storage.ownersKeptOffSync() == [first, second])
+
+        storage.releaseOwnersKeptOffSync([first])
+        #expect(storage.ownersKeptOffSync() == [second])
+        storage.clearAll()
+        #expect(storage.ownersKeptOffSync() == [second])
+    }
+
+    @Test("Removing tombstones by a predicate reaches every type and leaves the rest")
+    func removingOwnedTombstonesKeepsTheRest() {
+        let storage = makeStorage()
+        let removed = UUID()
+        let kept = UUID()
+        storage.addTombstones(["a"], type: .tableFavorite, owner: removed)
+        storage.addTombstones(["b"], type: .favorite, owner: removed)
+        storage.addTombstones(["c"], type: .tableFavorite, owner: kept)
+        storage.addTombstone("d", type: .tag)
+
+        storage.removeTombstones { _, tombstone in tombstone.owner == removed }
+
+        #expect(storage.tombstones(for: .tableFavorite).map(\.id) == ["c"])
+        #expect(storage.tombstones(for: .favorite).isEmpty)
+        #expect(storage.tombstones(for: .tag).map(\.id) == ["d"])
+    }
+
     @Test("The last sync date round-trips")
     func lastSyncDateRoundTrips() {
         let storage = makeStorage()

TablePro/AppDelegate.swift

@@ -136,6 +136,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         guard !AppStorageEnvironment.shared.isIsolated else { return }
 
         ConnectionStorage.shared.migratePluginSecureFieldsIfNeeded()
+        FavoriteTablesStorage.shared.migrateSyncIdentityIfNeeded()
         SoftwareUpdater.shared.start()
         AnalyticsService.shared.startPeriodicHeartbeat()
         SyncCoordinator.shared.start()

TablePro/Core/Services/Query/CatalogEditAdoption.swift

@@ -44,17 +44,23 @@ struct CatalogEditAdoption {
     private let schemaService: SchemaService
     private let connectionStorage: ConnectionStorage
     private let appSettings: AppSettingsStorage
+    private let favoriteTables: FavoriteTablesStorage
+    private let favoriteDatabases: FavoriteDatabasesStorage
 
     init(
         databaseManager: DatabaseManager = .shared,
         schemaService: SchemaService = .shared,
         connectionStorage: ConnectionStorage = .shared,
-        appSettings: AppSettingsStorage = .shared
+        appSettings: AppSettingsStorage = .shared,
+        favoriteTables: FavoriteTablesStorage = .shared,
+        favoriteDatabases: FavoriteDatabasesStorage = .shared
     ) {
         self.databaseManager = databaseManager
         self.schemaService = schemaService
         self.connectionStorage = connectionStorage
         self.appSettings = appSettings
+        self.favoriteTables = favoriteTables
+        self.favoriteDatabases = favoriteDatabases
     }
 
     /// Where the object lives. A reference without a database means the one being browsed, and the
@@ -81,12 +87,11 @@ struct CatalogEditAdoption {
     func adoptDroppedTables(_ refs: [DatabaseTreeTableRef], connectionId: UUID) {
         let dropped = Set(refs)
         updatePendingOperations(connectionId: connectionId) { dropped.contains($0) ? nil : $0 }
+        let droppedFavorites = Set(refs.map { favoriteEntry(for: $0, connectionId: connectionId) })
+        favoriteTables.retarget(connectionId: connectionId) { droppedFavorites.contains($0) ? nil : $0 }
         let sidebarState = SharedSidebarState.forConnection(connectionId)
         for ref in refs {
             sidebarState.removeRecentTable(database: ref.database, schema: ref.schema, name: ref.table.name)
-            FavoriteTablesStorage.shared.removeFavorite(
-                name: ref.table.name, schema: ref.favoriteSchema, database: ref.database, connectionId: connectionId
-            )
             guard let scope = objectScope(for: ref, connectionId: connectionId) else { continue }
             let tableScope = TableScope(
                 connectionId: connectionId, database: scope.database, schema: scope.schema, table: ref.table.name
@@ -107,7 +112,11 @@ struct CatalogEditAdoption {
         for store in TableScopedSettingsRegistry.stores {
             store.renameTable(from: oldScope, to: newScope)
         }
-        moveFavorite(ref, to: newName, connectionId: connectionId)
+        let oldFavorite = favoriteEntry(for: ref, connectionId: connectionId)
+        let newFavorite = FavoriteTablesStorage.FavoriteEntry(
+            connectionId: connectionId, database: ref.database, schema: ref.favoriteSchema, name: newName
+        )
+        favoriteTables.retarget(connectionId: connectionId) { $0 == oldFavorite ? newFavorite : $0 }
         SharedSidebarState.forConnection(connectionId).renameRecentTable(
             database: ref.database, schema: ref.schema, from: ref.table.name, to: newName
         )
@@ -123,7 +132,7 @@ struct CatalogEditAdoption {
                 database: oldDatabase, schema: nil, toDatabase: newName, toSchema: nil, connectionId: connectionId
             )
             SharedSidebarState.forConnection(connectionId).renameRecentDatabase(from: oldDatabase, to: newName)
-            FavoriteDatabasesStorage.shared.rename(database: oldDatabase, to: newName, connectionId: connectionId)
+            favoriteDatabases.rename(database: oldDatabase, to: newName, connectionId: connectionId)
             retargetDatabaseFilter(from: oldDatabase, to: newName, connectionId: connectionId)
             retargetSavedConnectionDatabase(from: oldDatabase, to: newName, connectionId: connectionId)
             retargetBrowseCursor(session.connection, from: oldDatabase, to: newName)
@@ -157,9 +166,7 @@ struct CatalogEditAdoption {
         for store in TableScopedSettingsRegistry.stores {
             store.dropContainer(connectionId: connectionId, database: database, schema: schema)
         }
-        FavoriteTablesStorage.shared.removeFavorites(
-            inDatabase: database, schema: schema, connectionId: connectionId
-        )
+        favoriteTables.removeFavorites(inDatabase: database, schema: schema, connectionId: connectionId)
 
         let sidebarState = SharedSidebarState.forConnection(connectionId)
         /// A dropped schema takes its own Recent entries with it and leaves its siblings alone.
@@ -171,7 +178,7 @@ struct CatalogEditAdoption {
             return
         }
         guard container.kind == .database else { return }
-        FavoriteDatabasesStorage.shared.removeFavorite(database: database, connectionId: connectionId)
+        favoriteDatabases.removeFavorite(database: database, connectionId: connectionId)
         clearSavedConnectionDatabase(named: database, connectionId: connectionId)
         sidebarState.clearRecentTables(inDatabase: database)
         var selected = sidebarState.databaseFilterSelected
@@ -236,18 +243,13 @@ struct CatalogEditAdoption {
         }
     }
 
-    /// Reads and writes the entry with `favoriteSchema`, the spelling the only writer of a table
-    /// favorite uses. Asking with the row's own schema instead missed the entry outright in a
-    /// hierarchical tree, where the schema hangs on the node and not on the `TableInfo`, so a
-    /// renamed table silently lost its star.
-    private func moveFavorite(_ ref: DatabaseTreeTableRef, to newName: String, connectionId: UUID) {
-        let storage = FavoriteTablesStorage.shared
-        let schema = ref.favoriteSchema
-        guard storage.isFavorite(
-            name: ref.table.name, schema: schema, database: ref.database, connectionId: connectionId
-        ) else { return }
-        storage.removeFavorite(name: ref.table.name, schema: schema, database: ref.database, connectionId: connectionId)
-        storage.addFavorite(name: newName, schema: schema, database: ref.database, connectionId: connectionId)
+    private func favoriteEntry(
+        for ref: DatabaseTreeTableRef,
+        connectionId: UUID
+    ) -> FavoriteTablesStorage.FavoriteEntry {
+        FavoriteTablesStorage.FavoriteEntry(
+            connectionId: connectionId, database: ref.database, schema: ref.favoriteSchema, name: ref.table.name
+        )
     }
 
     private func retargetContainer(
@@ -272,17 +274,14 @@ struct CatalogEditAdoption {
                 toDatabase: toDatabase, toSchema: toSchema
             )
         }
-        let storage = FavoriteTablesStorage.shared
-        for entry in storage.favorites(for: connectionId) where entry.database == database {
-            if let schema, entry.schema != schema { continue }
-            storage.removeFavorite(
-                name: entry.name, schema: entry.schema, database: entry.database, connectionId: connectionId
-            )
-            storage.addFavorite(
-                name: entry.name,
-                schema: schema == nil ? entry.schema : toSchema,
+        favoriteTables.retarget(connectionId: connectionId) { entry in
+            guard entry.database == database else { return entry }
+            if let schema, entry.schema != schema { return entry }
+            return FavoriteTablesStorage.FavoriteEntry(
+                connectionId: connectionId,
                 database: toDatabase,
-                connectionId: connectionId
+                schema: schema == nil ? entry.schema : toSchema,
+                name: entry.name
             )
         }
     }