From b5c7512bf12a1dead9d7587029618c806cc0a023 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 23:18:14 +0700 Subject: [PATCH 1/2] fix(sidebar): write favorite tables once per change and give each its own iCloud record --- CHANGELOG.md | 4 + TablePro/AppDelegate.swift | 1 + .../Services/Query/CatalogEditAdoption.swift | 63 ++-- .../Core/Storage/ConnectionLocalState.swift | 19 +- .../Storage/FavoriteDatabasesStorage.swift | 127 +++----- .../Core/Storage/FavoriteTablesStorage.swift | 299 ++++++++++------- .../SyncCoordinator+PushCollection.swift | 138 ++++++-- .../SyncCoordinator+RemoteDeletions.swift | 16 +- TablePro/Core/Sync/SyncChangeTracker.swift | 7 +- TablePro/Core/Sync/SyncCoordinator.swift | 96 ++++-- TablePro/Core/Sync/SyncEchoGuard.swift | 13 +- TablePro/Core/Sync/SyncRecordMapper.swift | 9 +- .../DatabaseTreeOutlineCoordinator.swift | 2 +- .../CatalogEditFavoriteAdoptionTests.swift | 119 +++++++ .../Storage/FavoriteTablesStorageTests.swift | 264 ++++++++++++++- .../Core/Storage/SyncDirtyMarkingTests.swift | 2 +- .../Sync/FavoriteTableSyncIdentityTests.swift | 307 ++++++++++++++++++ .../Core/Sync/SyncCoordinatorEchoTests.swift | 181 +---------- .../Core/Sync/SyncPushSettlementTests.swift | 171 ++++++++++ .../SyncRecordMapperFavoriteTableTests.swift | 79 ++++- .../Helpers/SyncTestEnvironment.swift | 224 +++++++++++++ docs/features/table-operations.mdx | 2 + 22 files changed, 1680 insertions(+), 463 deletions(-) create mode 100644 TableProTests/Core/Services/Query/CatalogEditFavoriteAdoptionTests.swift create mode 100644 TableProTests/Core/Sync/FavoriteTableSyncIdentityTests.swift create mode 100644 TableProTests/Core/Sync/SyncPushSettlementTests.swift create mode 100644 TableProTests/Helpers/SyncTestEnvironment.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fe3868f2a..29b0b8a0eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Remote deletions of connections, groups, tags, SSH profiles and table favorites applied with their sync category off. - **Local only** connections taking edits and deletions made on another device. - 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. ## [0.76.1] - 2026-09-29 diff --git a/TablePro/AppDelegate.swift b/TablePro/AppDelegate.swift index f9d3061e80..217c77eb07 100644 --- a/TablePro/AppDelegate.swift +++ b/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() diff --git a/TablePro/Core/Services/Query/CatalogEditAdoption.swift b/TablePro/Core/Services/Query/CatalogEditAdoption.swift index d71e2326e2..b36d365937 100644 --- a/TablePro/Core/Services/Query/CatalogEditAdoption.swift +++ b/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 @@ -75,12 +81,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 @@ -101,7 +106,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 ) @@ -117,7 +126,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) @@ -151,9 +160,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. @@ -165,7 +172,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 @@ -230,18 +237,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( @@ -266,17 +268,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 ) } } diff --git a/TablePro/Core/Storage/ConnectionLocalState.swift b/TablePro/Core/Storage/ConnectionLocalState.swift index 866ee61a19..f6486158b2 100644 --- a/TablePro/Core/Storage/ConnectionLocalState.swift +++ b/TablePro/Core/Storage/ConnectionLocalState.swift @@ -27,6 +27,8 @@ internal enum ConnectionLocalState { origin: Origin, appSettings: AppSettingsStorage = .shared, tableScopedStores: [any TableScopedSettingsStore] = TableScopedSettingsRegistry.stores, + favoriteTables: FavoriteTablesStorage = .shared, + favoriteDatabases: FavoriteDatabasesStorage = .shared, sqlFavorites: SQLFavoriteManager = .shared, queryHistory: QueryHistoryManager = .shared, defaults: UserDefaults = AppStorageEnvironment.shared.defaults @@ -37,7 +39,7 @@ internal enum ConnectionLocalState { purgeLiveState(connectionId) appSettings.saveLastDatabase(nil, for: connectionId) appSettings.saveLastSchema(nil, for: connectionId) - purgeFavorites(connectionId, origin: origin) + purgeFavorites(connectionId, origin: origin, tables: favoriteTables, databases: favoriteDatabases) SidebarPersistenceKey.removeAll(connectionId: connectionId) RecentTablesStore.shared.removeEntries(for: connectionId) HistoryPanelPreferencesStorage.remove(for: connectionId) @@ -125,14 +127,19 @@ internal enum ConnectionLocalState { ConnectionDataCache.removeConnection(connectionId) } - private static func purgeFavorites(_ connectionId: UUID, origin: Origin) { + private static func purgeFavorites( + _ connectionId: UUID, + origin: Origin, + tables: FavoriteTablesStorage, + databases: FavoriteDatabasesStorage + ) { switch origin { case .local: - FavoriteTablesStorage.shared.removeFavorites(for: connectionId) - FavoriteDatabasesStorage.shared.removeFavorites(for: connectionId) + tables.removeFavorites(for: connectionId) + databases.removeFavorites(for: connectionId) case .remote: - FavoriteTablesStorage.shared.removeFavoritesWithoutSync(for: connectionId) - FavoriteDatabasesStorage.shared.removeFavoritesWithoutSync(for: connectionId) + tables.removeFavoritesWithoutSync(for: connectionId) + databases.removeFavoritesWithoutSync(for: connectionId) } } } diff --git a/TablePro/Core/Storage/FavoriteDatabasesStorage.swift b/TablePro/Core/Storage/FavoriteDatabasesStorage.swift index 6dad69db05..f7e9345d7b 100644 --- a/TablePro/Core/Storage/FavoriteDatabasesStorage.swift +++ b/TablePro/Core/Storage/FavoriteDatabasesStorage.swift @@ -60,46 +60,48 @@ internal final class FavoriteDatabasesStorage { database: database, environment: environment ) - notify(after: mutate { Self.upsert(entry, into: &$0) }) + commit(sync: .track) { Self.upsert(entry, into: &$0) } } internal func setFavoriteWithoutSync(_ entry: FavoriteDatabaseEntry) { - notify(after: mutate { Self.upsert(entry, into: &$0) }, skipSync: true) + commit(sync: .discard) { Self.upsert(entry, into: &$0) } } - /// A favourite follows its database's new name rather than being dropped, because the tag the - /// user put on it is about the database, not about what it is called. It is synced, so the - /// entry is written before the removal is announced. internal func rename(database oldName: String, to newName: String, connectionId: UUID) { - guard let existing = favorites(for: connectionId).first(where: { $0.database == oldName }) else { return } - setFavorite(database: newName, environment: existing.environment, connectionId: connectionId) - removeFavorite(database: oldName, connectionId: connectionId) + commit(sync: .track) { favorites in + guard let existing = favorites.first(where: { + $0.connectionId == connectionId && $0.database == oldName + }) else { return } + favorites.remove(existing) + Self.upsert( + FavoriteDatabaseEntry(connectionId: connectionId, database: newName, environment: existing.environment), + into: &favorites + ) + } } internal func removeFavorite(database: String, connectionId: UUID) { - notify(after: mutate { favorites in - guard let existing = favorites.first(where: { - $0.connectionId == connectionId && $0.database == database - }) else { return .noChange } - favorites.remove(existing) - return .removed(existing) - }) + commit(sync: .track) { favorites in + favorites = favorites.filter { !($0.connectionId == connectionId && $0.database == database) } + } } internal func removeFavoriteWithoutSync(id: String) { - notify(after: mutate { favorites in - guard let entry = favorites.first(where: { Self.syncId(for: $0) == id }) else { return .noChange } - favorites.remove(entry) - return .removed(entry) - }, skipSync: true) + commit(sync: .discard) { favorites in + favorites = favorites.filter { Self.syncId(for: $0) != id } + } } internal func removeFavorites(for connectionId: UUID) { - removeFavorites(for: connectionId, skipSync: false) + commit(sync: .track) { favorites in + favorites = favorites.filter { $0.connectionId != connectionId } + } } internal func removeFavoritesWithoutSync(for connectionId: UUID) { - removeFavorites(for: connectionId, skipSync: true) + commit(sync: .discard) { favorites in + favorites = favorites.filter { $0.connectionId != connectionId } + } } /// The composite id never includes the environment. A record keyed on a mutable payload is @@ -109,74 +111,39 @@ internal final class FavoriteDatabasesStorage { (entry.connectionId.uuidString + "|" + entry.database).sha256 } - private func removeFavorites(for connectionId: UUID, skipSync: Bool) { - var favorites = loadFavorites() - let removed = favorites.filter { $0.connectionId == connectionId } - guard !removed.isEmpty else { return } - favorites.subtract(removed) - persist(favorites) - - guard !skipSync else { - syncTracker.discardDirty(.favoriteDatabase, ids: removed.map(Self.syncId(for:))) - postChangeNotification() - return - } - for entry in removed { - syncTracker.markDeleted(.favoriteDatabase, id: Self.syncId(for: entry)) - } - postChangeNotification() - } - - private enum TrackedAction { - case noChange - case changed(FavoriteDatabaseEntry) - case removed(FavoriteDatabaseEntry) + private enum SyncTracking { + case track + case discard } - /// Re-picking the environment a database already has is not a change. Persisting it anyway - /// posts a notification that rebuilds every visible tree row in every window for nothing. - private static func upsert( - _ entry: FavoriteDatabaseEntry, - into favorites: inout Set - ) -> TrackedAction { - guard !entry.database.isEmpty else { return .noChange } + private static func upsert(_ entry: FavoriteDatabaseEntry, into favorites: inout Set) { + guard !entry.database.isEmpty else { return } if let existing = favorites.first(where: { $0.id == entry.id }) { - guard existing.environment != entry.environment else { return .noChange } + guard existing.environment != entry.environment else { return } favorites.remove(existing) } favorites.insert(entry) - return .changed(entry) } - private func mutate(_ block: (inout Set) -> TrackedAction) -> TrackedAction { - var favorites = loadFavorites() - let action = block(&favorites) - guard case .noChange = action else { - persist(favorites) - return action - } - return action - } + private func commit(sync: SyncTracking, _ edit: (inout Set) -> Void) { + let previous = loadFavorites() + var favorites = previous + edit(&favorites) + let previousById = Dictionary(previous.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) + let currentIds = Set(favorites.map(\.id)) + let removedIds = previous.filter { !currentIds.contains($0.id) }.map(Self.syncId(for:)) + let changedIds = favorites.filter { previousById[$0.id] != $0 }.map(Self.syncId(for:)) + guard !removedIds.isEmpty || !changedIds.isEmpty else { return } - /// Persist first, then notify: `markDeleted` posts a change that can start a sync, and a sync - /// that reads a file still holding the deleted entry re-uploads it. - private func notify(after action: TrackedAction, skipSync: Bool = false) { - switch action { - case .noChange: - return - case .changed(let entry): - if !skipSync { - syncTracker.markDirty(.favoriteDatabase, id: Self.syncId(for: entry)) - } - postChangeNotification() - case .removed(let entry): - if skipSync { - syncTracker.discardDirty(.favoriteDatabase, ids: [Self.syncId(for: entry)]) - } else { - syncTracker.markDeleted(.favoriteDatabase, id: Self.syncId(for: entry)) - } - postChangeNotification() + persist(favorites) + switch sync { + case .track: + syncTracker.markDeleted(.favoriteDatabase, ids: removedIds) + syncTracker.markDirty(.favoriteDatabase, ids: changedIds) + case .discard: + syncTracker.discardDirty(.favoriteDatabase, ids: removedIds) } + postChangeNotification() } private func postChangeNotification() { diff --git a/TablePro/Core/Storage/FavoriteTablesStorage.swift b/TablePro/Core/Storage/FavoriteTablesStorage.swift index 021d73fee7..fe5f9ddaf4 100644 --- a/TablePro/Core/Storage/FavoriteTablesStorage.swift +++ b/TablePro/Core/Storage/FavoriteTablesStorage.swift @@ -1,5 +1,6 @@ import Foundation import os +import TableProPluginKit import TableProSyncTransport extension Notification.Name { @@ -9,17 +10,56 @@ extension Notification.Name { final class FavoriteTablesStorage: @unchecked Sendable { static let shared = FavoriteTablesStorage() private static let logger = Logger(subsystem: "com.TablePro", category: "FavoriteTablesStorage") + private static let currentSyncIdentityVersion = 1 + private static let escapedPathDomain = "escaped" struct FavoriteEntry: Codable, Hashable { let connectionId: UUID let database: String? let schema: String? let name: String + + init(connectionId: UUID, database: String?, schema: String?, name: String) { + self.connectionId = connectionId + self.database = database?.nilIfEmpty + self.schema = schema?.nilIfEmpty + self.name = name + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.init( + connectionId: try container.decode(UUID.self, forKey: .connectionId), + database: try container.decodeIfPresent(String.self, forKey: .database), + schema: try container.decodeIfPresent(String.self, forKey: .schema), + name: try container.decode(String.self, forKey: .name) + ) + } + + private enum CodingKeys: String, CodingKey { + case connectionId + case database + case schema + case name + } + } + + private enum SyncTracking { + case track + case discard + } + + private struct StateChange { + var removed: Set = [] + var added: Set = [] + + var changesEntries: Bool { !removed.isEmpty || !added.isEmpty } } private let defaults: UserDefaults private let syncTracker: SyncChangeTracker private let key = "com.TablePro.favoriteTables" + private let syncIdentityVersionKey = "com.TablePro.favoriteTables.syncIdentityVersion" private var cache: Set? private let lock = NSLock() @@ -51,185 +91,212 @@ final class FavoriteTablesStorage: @unchecked Sendable { @MainActor func toggle(name: String, schema: String?, database: String?, connectionId: UUID) { let entry = FavoriteEntry(connectionId: connectionId, database: database, schema: schema, name: name) - let action: TrackedAction = mutate { favorites in - if favorites.contains(entry) { - favorites.remove(entry) - return .removed(entry) - } + commit(sync: .track) { favorites in + guard favorites.remove(entry) == nil else { return } favorites.insert(entry) - return .added(entry) } - notify(after: action) } @MainActor @discardableResult func addFavorite(name: String, schema: String?, database: String?, connectionId: UUID) -> Bool { let entry = FavoriteEntry(connectionId: connectionId, database: database, schema: schema, name: name) - let action: TrackedAction = mutate { favorites in - guard favorites.insert(entry).inserted else { return .noChange } - return .added(entry) - } - notify(after: action) - return action.changed - } - - @MainActor - @discardableResult - func addFavoriteWithoutSync(_ entry: FavoriteEntry) -> Bool { - let action = mutate { favorites in - favorites.insert(entry).inserted ? .added(entry) : .noChange - } - notify(after: action, skipSync: true) - return action.changed + return commit(sync: .track) { $0.insert(entry) }.changesEntries } @MainActor func removeFavorite(name: String, schema: String?, database: String?, connectionId: UUID) { let entry = FavoriteEntry(connectionId: connectionId, database: database, schema: schema, name: name) - let action = mutate { favorites in - favorites.remove(entry) != nil ? .removed(entry) : .noChange - } - notify(after: action) - } - - @MainActor - func removeFavoriteWithoutSync(_ entry: FavoriteEntry) { - let action = mutate { favorites in - favorites.remove(entry) != nil ? .removed(entry) : .noChange - } - notify(after: action, skipSync: true) + commit(sync: .track) { $0.remove(entry) } } @MainActor - func removeFavoriteWithoutSync(id: String) { - let action = mutate { favorites in - guard let entry = favorites.first(where: { Self.syncId(for: $0) == id }) else { return .noChange } - favorites.remove(entry) - return .removed(entry) + func retarget(connectionId: UUID, _ transform: (FavoriteEntry) -> FavoriteEntry?) { + commit(sync: .track) { favorites in + let scoped = favorites.filter { $0.connectionId == connectionId } + favorites.subtract(scoped) + favorites.formUnion(scoped.compactMap(transform)) } - notify(after: action, skipSync: true) - syncTracker.discardDirty(.tableFavorite, ids: [id]) } - /// Drops every favorite inside a database, or inside one schema of it when a schema is named. - /// - /// The container is gone, so each entry names a table that no longer exists. Removed one at a - /// time through the syncing path, because the tables are gone on every device, not only this one. @MainActor @discardableResult func removeFavorites(inDatabase database: String?, schema: String?, connectionId: UUID) -> [FavoriteEntry] { - let doomed = favorites(for: connectionId).filter { entry in - guard entry.database == database else { return false } - guard let schema else { return true } - return entry.schema == schema - } - for entry in doomed { - removeFavorite( - name: entry.name, schema: entry.schema, database: entry.database, connectionId: connectionId - ) + let database = database?.nilIfEmpty + let schema = schema?.nilIfEmpty + let change = commit(sync: .track) { favorites in + favorites = favorites.filter { entry in + guard entry.connectionId == connectionId, entry.database == database else { return true } + guard let schema else { return false } + return entry.schema != schema + } } - return Array(doomed) + return Array(change.removed) } @MainActor @discardableResult func removeFavorites(for connectionId: UUID) -> [FavoriteEntry] { - removeFavorites(for: connectionId, skipSync: false) + removeFavorites(for: connectionId, sync: .track) } - /// Used when another device deleted the connection. Marking tombstones here would push its own - /// deletion straight back at it. @MainActor @discardableResult func removeFavoritesWithoutSync(for connectionId: UUID) -> [FavoriteEntry] { - removeFavorites(for: connectionId, skipSync: true) + removeFavorites(for: connectionId, sync: .discard) } @MainActor - @discardableResult - private func removeFavorites(for connectionId: UUID, skipSync: Bool) -> [FavoriteEntry] { - var removed: [FavoriteEntry] = [] - lock.lock() - var favorites = _loadFavorites() - let toRemove = favorites.filter { $0.connectionId == connectionId } - if !toRemove.isEmpty { - favorites.subtract(toRemove) - _persist(favorites) - removed = Array(toRemove) + private func removeFavorites(for connectionId: UUID, sync: SyncTracking) -> [FavoriteEntry] { + let change = commit(sync: sync) { favorites in + favorites = favorites.filter { entry in entry.connectionId != connectionId } } - lock.unlock() + return Array(change.removed) + } - guard !removed.isEmpty else { return [] } - if skipSync { - syncTracker.discardDirty(.tableFavorite, ids: removed.map(Self.syncId(for:))) - } else { - for entry in removed { - syncTracker.markDeleted(.tableFavorite, id: Self.syncId(for: entry)) - } + @MainActor + @discardableResult + func applyRemote(saved: [FavoriteEntry], deletedIds: Set) -> Set { + var removedThroughAliases: Set = [] + let change = mutateState { favorites in + favorites.formUnion(saved) + removedThroughAliases = Self.remove(deletedIds, from: &favorites) + } + syncTracker.discardDirty(.tableFavorite, ids: Array(deletedIds.union(Self.recordIds(of: change.removed)))) + if change.changesEntries { + NotificationCenter.default.post(name: .favoriteTablesDidChange, object: self) } - NotificationCenter.default.post(name: .favoriteTablesDidChange, object: nil) - return removed + return Set(removedThroughAliases.map(Self.syncId(for:))) + } + + @MainActor + func migrateSyncIdentityIfNeeded() { + guard defaults.integer(forKey: syncIdentityVersionKey) < Self.currentSyncIdentityVersion else { return } + let rekeyed = loadFavorites().filter { Self.legacyAlias(of: $0) != nil } + let rekeyedIds = Array(Self.recordIds(of: rekeyed)) + syncTracker.markDirty(.tableFavorite, ids: rekeyedIds) + defaults.set(Self.currentSyncIdentityVersion, forKey: syncIdentityVersionKey) + guard !rekeyed.isEmpty else { return } + Self.logger.info("Re-keyed \(rekeyed.count, privacy: .public) favorite tables to escaped sync ids") } static func syncId(for entry: FavoriteEntry) -> String { - let raw = entry.connectionId.uuidString - + "|" + (entry.database ?? "") - + "|" + (entry.schema ?? "") - + "|" + entry.name - return raw.sha256 + let path = IdentityPath.joined(identityComponents(of: entry), separator: "|") + guard path != legacyPath(of: entry) else { return path.sha256 } + return IdentityPath.joined([escapedPathDomain, path], separator: "|").sha256 + } + + static func legacyAlias(of entry: FavoriteEntry) -> String? { + let legacy = legacyPath(of: entry) + guard IdentityPath.joined(identityComponents(of: entry), separator: "|") != legacy else { return nil } + return legacy.sha256 } - private enum TrackedAction { - case noChange - case added(FavoriteEntry) - case removed(FavoriteEntry) + static func legacyAliases(of favorites: Set) -> Set { + Set(favorites.compactMap(legacyAlias(of:))) + } - var changed: Bool { - if case .noChange = self { return false } - return true + static func aliasClaims(in favorites: Set) -> [String: [FavoriteEntry]] { + var claims: [String: [FavoriteEntry]] = [:] + for entry in favorites { + guard let alias = legacyAlias(of: entry) else { continue } + claims[alias, default: []].append(entry) } + return claims } - private func mutate(_ block: (inout Set) -> TrackedAction) -> TrackedAction { + private static func identityComponents(of entry: FavoriteEntry) -> [String] { + [entry.connectionId.uuidString, entry.database ?? "", entry.schema ?? "", entry.name] + } + + private static func legacyPath(of entry: FavoriteEntry) -> String { + identityComponents(of: entry).joined(separator: "|") + } + + @MainActor + @discardableResult + private func commit(sync: SyncTracking, _ edit: (inout Set) -> Void) -> StateChange { + var remaining: Set = [] + let change = mutateState { favorites in + edit(&favorites) + remaining = favorites + } + guard change.changesEntries else { return change } + let removedIds = Self.recordIds(of: change.removed, keepingAliasesOf: remaining) + switch sync { + case .track: + let reclaimedAliases = Self.legacyAliases(of: change.removed).intersection(Self.legacyAliases(of: remaining)) + syncTracker.markDeleted(.tableFavorite, ids: Array(removedIds)) + syncTracker.markDirty(.tableFavorite, ids: Array(Self.recordIds(of: change.added).union(reclaimedAliases))) + case .discard: + syncTracker.discardDirty(.tableFavorite, ids: Array(removedIds)) + } + NotificationCenter.default.post(name: .favoriteTablesDidChange, object: self) + return change + } + + @discardableResult + private func mutateState(_ edit: (inout Set) -> Void) -> StateChange { lock.lock() defer { lock.unlock() } - var favorites = _loadFavorites() - let action = block(&favorites) - guard action.changed else { return action } - _persist(favorites) - return action + let previous = _loadFavorites() + var favorites = previous + edit(&favorites) + let change = StateChange(removed: previous.subtracting(favorites), added: favorites.subtracting(previous)) + if change.changesEntries { + _persist(favorites) + } + return change } - @MainActor - private func notify(after action: TrackedAction, skipSync: Bool = false) { - switch action { - case .noChange: - return - case .added(let entry): - if !skipSync { - syncTracker.markDirty(.tableFavorite, id: Self.syncId(for: entry)) + private static func recordIds(of favorites: Set) -> Set { + Set(favorites.map(syncId(for:))).union(legacyAliases(of: favorites)) + } + + private static func recordIds( + of removed: Set, + keepingAliasesOf remaining: Set + ) -> Set { + let claimedAliases = legacyAliases(of: remaining) + let retiredAliases = legacyAliases(of: removed).subtracting(claimedAliases) + return Set(removed.map(syncId(for:))).union(retiredAliases) + } + + private static func remove(_ deletedIds: Set, from favorites: inout Set) -> Set { + guard !deletedIds.isEmpty else { return [] } + let entriesById = Dictionary(favorites.map { (syncId(for: $0), $0) }, uniquingKeysWith: { first, _ in first }) + let claims = aliasClaims(in: favorites) + var removedThroughAliases: Set = [] + for id in deletedIds { + if let entry = entriesById[id] { + favorites.remove(entry) + continue } - NotificationCenter.default.post(name: .favoriteTablesDidChange, object: nil) - case .removed(let entry): - if skipSync { - syncTracker.discardDirty(.tableFavorite, ids: [Self.syncId(for: entry)]) - } else { - syncTracker.markDeleted(.tableFavorite, id: Self.syncId(for: entry)) + guard let claimants = claims[id] else { continue } + guard claimants.count == 1, let entry = claimants.first else { + logger.warning("Kept \(claimants.count, privacy: .public) favorite tables sharing one retired sync id") + continue } - NotificationCenter.default.post(name: .favoriteTablesDidChange, object: nil) + favorites.remove(entry) + removedThroughAliases.insert(entry) } + return removedThroughAliases.filter { !deletedIds.contains(syncId(for: $0)) } } private func _loadFavorites() -> Set { if let cache { return cache } guard let data = defaults.data(forKey: key), - let decoded = try? JSONDecoder().decode(Set.self, from: data) else { + let stored = try? JSONDecoder().decode([FavoriteEntry].self, from: data) else { cache = [] return [] } - cache = decoded - return decoded + let favorites = Set(stored) + guard favorites.count == stored.count else { + _persist(favorites) + return favorites + } + cache = favorites + return favorites } private func _persist(_ favorites: Set) { diff --git a/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift b/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift index 04f0e2d1d0..f35bef2c21 100644 --- a/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift +++ b/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift @@ -6,12 +6,18 @@ import TableProSyncTransport struct SyncPushBatch { var records: [CKRecord] = [] var deletions: [CKRecord.ID] = [] + var supersededTombstones: Set = [] var uniqueDeletions: [CKRecord.ID] { Array(Set(deletions)) } } +private struct BuiltRecord { + let id: String + let record: CKRecord +} + extension SyncCoordinator { func collectPushBatch( snapshot: SyncEditSnapshot, @@ -89,18 +95,51 @@ extension SyncCoordinator { loaded: () async -> [Record]?, record: (Record) -> CKRecord? ) async where Record.ID == UUID { - appendTombstones(of: type, to: &batch, zoneID: zoneID) let dirtyIds = snapshot.dirtyIds(for: type) - guard !dirtyIds.isEmpty, let records = await loaded() else { return } + guard !dirtyIds.isEmpty else { + append(type, dirtyIds: dirtyIds, built: [], into: &batch, zoneID: zoneID) + return + } + let built = await loaded().map { records in + Self.build(records, dirtyIds: dirtyIds, id: { $0.id.uuidString }, record: record) + } + append(type, dirtyIds: dirtyIds, built: built, into: &batch, zoneID: zoneID) + } + + private static func build( + _ items: [Item], + dirtyIds: Set, + id: (Item) -> String, + record: (Item) -> CKRecord? + ) -> [BuiltRecord] { + var built: [BuiltRecord] = [] + var builtIds: Set = [] + for item in items { + let itemId = id(item) + guard dirtyIds.contains(itemId), !builtIds.contains(itemId), let record = record(item) else { continue } + builtIds.insert(itemId) + built.append(BuiltRecord(id: itemId, record: record)) + } + return built + } - var pushable: Set = [] - for item in records where dirtyIds.contains(item.id.uuidString) { - let id = item.id.uuidString - guard !pushable.contains(id), let built = record(item) else { continue } - pushable.insert(id) - batch.records.append(built) + private func append( + _ type: SyncRecordType, + dirtyIds: Set, + built: [BuiltRecord]?, + retaining retainedIds: Set = [], + into batch: inout SyncPushBatch, + zoneID: CKRecordZone.ID + ) { + guard let built else { + appendTombstones(of: type, sparing: dirtyIds, to: &batch, zoneID: zoneID) + return } - discardUnpushable(type, dirtyIds: dirtyIds, pushable: pushable) + let pushable = Set(built.map(\.id)) + batch.records.append(contentsOf: built.map(\.record)) + discardUnpushable(type, dirtyIds: dirtyIds.subtracting(retainedIds), pushable: pushable) + let superseded = appendTombstones(of: type, sparing: pushable, to: &batch, zoneID: zoneID) + batch.supersededTombstones.formUnion(superseded.map { SyncRecordIdentity(type: type, id: $0) }) } private func discardUnpushable(_ type: SyncRecordType, dirtyIds: Set, pushable: Set) { @@ -112,10 +151,22 @@ extension SyncCoordinator { changeTracker.discardDirty(type, ids: Array(unpushable)) } - private func appendTombstones(of type: SyncRecordType, to batch: inout SyncPushBatch, zoneID: CKRecordZone.ID) { + @discardableResult + private func appendTombstones( + of type: SyncRecordType, + sparing sparedIds: Set, + to batch: inout SyncPushBatch, + zoneID: CKRecordZone.ID + ) -> Set { + var spared: Set = [] for tombstone in metadataStorage.tombstones(for: type) { + guard !sparedIds.contains(tombstone.id) else { + spared.insert(tombstone.id) + continue + } batch.deletions.append(SyncRecordMapper.recordID(type: type, id: tombstone.id, in: zoneID)) } + return spared } private func collectSettings(snapshot: SyncEditSnapshot, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID) { @@ -130,13 +181,43 @@ extension SyncCoordinator { into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) { - appendTombstones(of: .tableFavorite, to: &batch, zoneID: zoneID) let dirtyIds = snapshot.dirtyIds(for: .tableFavorite) - guard !dirtyIds.isEmpty else { return } - for entry in services.favoriteTablesStorage.loadFavorites() - where dirtyIds.contains(FavoriteTablesStorage.syncId(for: entry)) { - batch.records.append(SyncRecordMapper.toCKRecord(favoriteEntry: entry, in: zoneID)) + guard !dirtyIds.isEmpty else { + append(.tableFavorite, dirtyIds: dirtyIds, built: [], into: &batch, zoneID: zoneID) + return + } + let built = Self.buildTableFavorites( + services.favoriteTablesStorage.loadFavorites(), + dirtyIds: dirtyIds, + zoneID: zoneID + ) + append(.tableFavorite, dirtyIds: dirtyIds, built: built, into: &batch, zoneID: zoneID) + } + + private static func buildTableFavorites( + _ favorites: Set, + dirtyIds: Set, + zoneID: CKRecordZone.ID + ) -> [BuiltRecord] { + let claims = FavoriteTablesStorage.aliasClaims(in: favorites) + var built: [BuiltRecord] = [] + for entry in favorites { + let currentId = FavoriteTablesStorage.syncId(for: entry) + if dirtyIds.contains(currentId) { + built.append(BuiltRecord( + id: currentId, + record: SyncRecordMapper.toCKRecord(favoriteEntry: entry, recordId: currentId, in: zoneID) + )) + } + guard let alias = FavoriteTablesStorage.legacyAlias(of: entry), + dirtyIds.contains(alias), + claims[alias]?.count == 1 else { continue } + built.append(BuiltRecord( + id: alias, + record: SyncRecordMapper.toCKRecord(favoriteEntry: entry, recordId: alias, in: zoneID) + )) } + return built } /// A connection the user marked local only never reaches iCloud, and neither do the database @@ -147,15 +228,28 @@ extension SyncCoordinator { into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) { - appendTombstones(of: .favoriteDatabase, to: &batch, zoneID: zoneID) let dirtyIds = snapshot.dirtyIds(for: .favoriteDatabase) - guard !dirtyIds.isEmpty else { return } - let localOnlyIds = Set(services.connectionStorage.loadConnections().filter(\.localOnly).map(\.id)) - for entry in services.favoriteDatabasesStorage.loadFavorites() - where dirtyIds.contains(FavoriteDatabasesStorage.syncId(for: entry)) - && !localOnlyIds.contains(entry.connectionId) { - batch.records.append(SyncRecordMapper.toCKRecord(favoriteDatabase: entry, in: zoneID)) + guard !dirtyIds.isEmpty else { + append(.favoriteDatabase, dirtyIds: dirtyIds, built: [], into: &batch, zoneID: zoneID) + return } + let localOnlyIds = Set(services.connectionStorage.loadConnections().filter(\.localOnly).map(\.id)) + let favorites = services.favoriteDatabasesStorage.loadFavorites() + let withheld = favorites.filter { localOnlyIds.contains($0.connectionId) } + let built = Self.build( + Array(favorites.subtracting(withheld)), + dirtyIds: dirtyIds, + id: FavoriteDatabasesStorage.syncId(for:), + record: { SyncRecordMapper.toCKRecord(favoriteDatabase: $0, in: zoneID) } + ) + append( + .favoriteDatabase, + dirtyIds: dirtyIds, + built: built, + retaining: Set(withheld.map(FavoriteDatabasesStorage.syncId(for:))), + into: &batch, + zoneID: zoneID + ) } private func collectSQLFavorites( diff --git a/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift b/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift index 8d641fb2fa..a3b12d043d 100644 --- a/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift +++ b/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift @@ -52,13 +52,21 @@ struct SyncRemoteDeletionEffects { var connectionsChanged = false var groupsOrTagsChanged = false var persistenceFailed = false + var tableFavoriteIdsToRetire: Set = [] } extension SyncCoordinator { - func applyRemoteDeletions(_ pending: SyncPendingDeletions) -> SyncRemoteDeletionEffects { + func applyRemoteDeletions( + _ pending: SyncPendingDeletions, + alongside tableFavorites: [FavoriteTablesStorage.FavoriteEntry] + ) -> SyncRemoteDeletionEffects { var effects = SyncRemoteDeletionEffects() effects.connectionsChanged = !pending.connections.isEmpty effects.groupsOrTagsChanged = !pending.groups.isEmpty || !pending.tags.isEmpty + effects.tableFavoriteIdsToRetire = services.favoriteTablesStorage.applyRemote( + saved: tableFavorites, + deletedIds: pending.tableFavorites + ) let persisted = [ applyRemoteConnectionDeletions(pending.connections), @@ -68,10 +76,6 @@ extension SyncCoordinator { applyRemoteCredentialProfileDeletions(pending.credentialProfiles) ] effects.persistenceFailed = persisted.contains(false) - - for id in pending.tableFavorites { - services.favoriteTablesStorage.removeFavoriteWithoutSync(id: id) - } return effects } @@ -89,6 +93,8 @@ extension SyncCoordinator { ConnectionLocalState.purge( connectionIds: deletedIds, origin: .remote, + favoriteTables: services.favoriteTablesStorage, + favoriteDatabases: services.favoriteDatabasesStorage, sqlFavorites: services.sqlFavoriteManager, queryHistory: services.queryHistoryManager ) diff --git a/TablePro/Core/Sync/SyncChangeTracker.swift b/TablePro/Core/Sync/SyncChangeTracker.swift index 5eb1ee9bb1..2faea5c48a 100644 --- a/TablePro/Core/Sync/SyncChangeTracker.swift +++ b/TablePro/Core/Sync/SyncChangeTracker.swift @@ -103,10 +103,15 @@ final class SyncChangeTracker: Sendable { let dirty = Set(SyncRecordType.allCases.flatMap { type in metadataStorage.dirtyIds(for: type).map { SyncRecordIdentity(type: type, id: $0) } }) + let tombstoned = Set(SyncRecordType.allCases.flatMap { type in + metadataStorage.tombstones(for: type).map { SyncRecordIdentity(type: type, id: $0.id) } + }) return editGenerations.withLock { state in SyncEditSnapshot( dirty: dirty, - generations: Dictionary(uniqueKeysWithValues: dirty.map { ($0, state.generation(of: $0)) }) + generations: Dictionary( + uniqueKeysWithValues: dirty.union(tombstoned).map { ($0, state.generation(of: $0)) } + ) ) } } diff --git a/TablePro/Core/Sync/SyncCoordinator.swift b/TablePro/Core/Sync/SyncCoordinator.swift index 99fa7c37cc..552a80e134 100644 --- a/TablePro/Core/Sync/SyncCoordinator.swift +++ b/TablePro/Core/Sync/SyncCoordinator.swift @@ -390,7 +390,7 @@ final class SyncCoordinator: ObservableObject { guard !batch.records.isEmpty || !deletions.isEmpty else { return PushReport() } let identities = SyncRecordMapper.identities(for: pushedLocalIds(snapshot), in: zoneID) - let outcome: PushOutcome + var outcome: PushOutcome var interruption: Error? do { outcome = try await transport.push(records: batch.records, deletions: deletions) @@ -401,14 +401,17 @@ final class SyncCoordinator: ObservableObject { Self.logger.error("Push failed: \(error.localizedDescription)") return PushReport(error: error) } + outcome.acceptMissingDeletions(of: deletions) recordCache.store(Array(outcome.savedRecords.values)) recordCache.remove(Array(outcome.deletedRecordIDs)) let savedRecords = settleSavedRecords(outcome, batch: batch, identities: identities, snapshot: snapshot) + var deletedRecords: [CKRecord.ID: SyncRecordIdentity] = [:] for recordID in outcome.deletedRecordIDs { guard let identity = identities[recordID] else { continue } + deletedRecords[recordID] = identity metadataStorage.removeTombstone(identity.id, type: identity.type) } @@ -417,7 +420,7 @@ final class SyncCoordinator: ObservableObject { let rejectedCount = outcome.failures.count Self.logger.info("Push completed: \(savedCount) saved, \(deletedCount) deleted, \(rejectedCount) rejected") - let echoGuard = SyncEchoGuard(snapshot: snapshot, savedRecords: savedRecords) + let echoGuard = SyncEchoGuard(snapshot: snapshot, savedRecords: savedRecords, deletedRecords: deletedRecords) if let interruption { Self.logger.error("Push stopped part way: \(interruption.localizedDescription)") return PushReport(echoGuard: echoGuard, error: interruption) @@ -440,7 +443,11 @@ final class SyncCoordinator: ObservableObject { for recordID in outcome.savedRecords.keys { guard let identity = identities[recordID] else { continue } savedRecords[recordID] = identity - changeTracker.clearDirty(identity, unlessEditedSince: snapshot) + guard !changeTracker.hasEdit(identity, since: snapshot) else { continue } + if batch.supersededTombstones.contains(identity) { + metadataStorage.removeTombstone(identity.id, type: identity.type) + } + changeTracker.clearDirty(identity.type, id: identity.id) } return savedRecords } @@ -490,9 +497,17 @@ final class SyncCoordinator: ObservableObject { @discardableResult internal func applyPullResult(_ result: PullResult, echoGuard: SyncEchoGuard? = nil) async -> Bool { let settings = services.appSettingsStorage.loadSync() - let storesPersisted = applyRemoteChanges(result, settings: settings, echoGuard: echoGuard) + let deletedRecordIDs = result.deletedRecordIDs.filter { recordID in + echoGuard?.withholdsDeletion(recordID, tracker: changeTracker) != true + } + let storesPersisted = applyRemoteChanges( + result, + deletedRecordIDs: deletedRecordIDs, + settings: settings, + echoGuard: echoGuard + ) let favoritesOutcome = await services.sqlFavoriteManager.applyRemote( - remoteSQLFavoriteBatch(from: result, settings: settings), + remoteSQLFavoriteBatch(from: result, deletedRecordIDs: deletedRecordIDs, settings: settings), echoGuard: echoGuard ) @@ -526,14 +541,33 @@ final class SyncCoordinator: ObservableObject { // for large payloads. /// Reports whether every record that can say so was persisted. A pull that answers false must /// not commit its token: the batch has to arrive again. - private func applyRemoteChanges(_ result: PullResult, settings: SyncSettings, echoGuard: SyncEchoGuard?) -> Bool { + private func applyRemoteChanges( + _ result: PullResult, + deletedRecordIDs: [CKRecord.ID], + settings: SyncSettings, + echoGuard: SyncEchoGuard? + ) -> Bool { services.connectionStorage.invalidateCache() changeTracker.isSuppressed = true - defer { - changeTracker.isSuppressed = false - } + let effects = applyRemoteRecords( + result.changedRecords, + deletedRecordIDs: deletedRecordIDs, + settings: settings, + echoGuard: echoGuard + ) + changeTracker.isSuppressed = false + + changeTracker.markDeleted(.tableFavorite, ids: Array(effects.tableFavoriteIdsToRetire)) + return !effects.persistenceFailed + } + private func applyRemoteRecords( + _ changedRecords: [CKRecord], + deletedRecordIDs: [CKRecord.ID], + settings: SyncSettings, + echoGuard: SyncEchoGuard? + ) -> SyncRemoteDeletionEffects { var actualConnectionChanges = false var groupsOrTagsChanged = false var persistenceFailed = false @@ -544,9 +578,10 @@ final class SyncCoordinator: ObservableObject { let sshTombstoneIds = Set(metadataStorage.tombstones(for: .sshProfile).map(\.id)) let credentialTombstoneIds = Set(metadataStorage.tombstones(for: .credentialProfile).map(\.id)) let tableFavoriteTombstoneIds = Set(metadataStorage.tombstones(for: .tableFavorite).map(\.id)) + var tableFavorites: [FavoriteTablesStorage.FavoriteEntry] = [] let databaseFavoriteTombstoneIds = Set(metadataStorage.tombstones(for: .favoriteDatabase).map(\.id)) - for record in result.changedRecords { + for record in changedRecords { if let echoGuard, echoGuard.withholds(record.recordID, tracker: changeTracker) { Self.logger.info("Kept a local edit made while its record was being pushed") continue @@ -580,7 +615,9 @@ final class SyncCoordinator: ObservableObject { case .settings: applyRemoteSettings(record) case .tableFavorite: - applyRemoteTableFavorite(record, tombstoneIds: tableFavoriteTombstoneIds) + if let favorite = remoteTableFavorite(record, tombstoneIds: tableFavoriteTombstoneIds) { + tableFavorites.append(favorite) + } case .favoriteDatabase: applyRemoteDatabaseFavorite(record, tombstoneIds: databaseFavoriteTombstoneIds) case .favorite, .favoriteFolder: @@ -588,10 +625,13 @@ final class SyncCoordinator: ObservableObject { } } - let deletions = applyRemoteDeletions(SyncPendingDeletions.parse(result.deletedRecordIDs, settings: settings)) - actualConnectionChanges = actualConnectionChanges || deletions.connectionsChanged - groupsOrTagsChanged = groupsOrTagsChanged || deletions.groupsOrTagsChanged - persistenceFailed = persistenceFailed || deletions.persistenceFailed + var effects = applyRemoteDeletions( + SyncPendingDeletions.parse(deletedRecordIDs, settings: settings), + alongside: tableFavorites + ) + actualConnectionChanges = actualConnectionChanges || effects.connectionsChanged + groupsOrTagsChanged = groupsOrTagsChanged || effects.groupsOrTagsChanged + effects.persistenceFailed = persistenceFailed || effects.persistenceFailed /// After the batch, never per record: a pull carries no dependency order, so a legal /// hierarchy change spread over two records passes through a state that reads as a cycle @@ -604,13 +644,17 @@ final class SyncCoordinator: ObservableObject { services.appEvents.connectionUpdated.send(nil) } - return !persistenceFailed + return effects } - private func remoteSQLFavoriteBatch(from result: PullResult, settings: SyncSettings) -> RemoteSQLFavoriteBatch { + private func remoteSQLFavoriteBatch( + from result: PullResult, + deletedRecordIDs: [CKRecord.ID], + settings: SyncSettings + ) -> RemoteSQLFavoriteBatch { guard settings.syncSQLFavorites else { return RemoteSQLFavoriteBatch() } - let deletions = SyncPendingDeletions.parse(result.deletedRecordIDs, settings: settings) + let deletions = SyncPendingDeletions.parse(deletedRecordIDs, settings: settings) var batch = RemoteSQLFavoriteBatch( deletedFavoriteIds: deletions.sqlFavorites, deletedFolderIds: deletions.sqlFolders @@ -794,20 +838,24 @@ final class SyncCoordinator: ObservableObject { } } - @discardableResult - private func applyRemoteTableFavorite(_ record: CKRecord, tombstoneIds: Set) -> Bool { + private func remoteTableFavorite( + _ record: CKRecord, + tombstoneIds: Set + ) -> FavoriteTablesStorage.FavoriteEntry? { + let recordName = record.recordID.recordName let entry: FavoriteTablesStorage.FavoriteEntry do { entry = try SyncRecordMapper.favoriteEntry(from: record) } catch { - let recordName = record.recordID.recordName Self.logger.error( "Skipping remote favorite table \(recordName, privacy: .private(mask: .hash)): \(error.publicLogShape, privacy: .public) \(error.localizedDescription, privacy: .private)" ) - return false + return nil } - if tombstoneIds.contains(FavoriteTablesStorage.syncId(for: entry)) { return false } - return services.favoriteTablesStorage.addFavoriteWithoutSync(entry) + guard let recordId = SyncRecordType.parse(recordName: recordName)?.id, + !tombstoneIds.contains(recordId), + !tombstoneIds.contains(FavoriteTablesStorage.syncId(for: entry)) else { return nil } + return entry } /// Upserts rather than inserts. A database favorite carries a mutable payload, the environment diff --git a/TablePro/Core/Sync/SyncEchoGuard.swift b/TablePro/Core/Sync/SyncEchoGuard.swift index 7a319c4947..4c9e90640c 100644 --- a/TablePro/Core/Sync/SyncEchoGuard.swift +++ b/TablePro/Core/Sync/SyncEchoGuard.swift @@ -7,15 +7,26 @@ struct SyncEchoGuard: Sendable { let snapshot: SyncEditSnapshot let savedRecords: [CKRecord.ID: SyncRecordIdentity] + let deletedRecords: [CKRecord.ID: SyncRecordIdentity] private let savedIdentities: Set - init(snapshot: SyncEditSnapshot, savedRecords: [CKRecord.ID: SyncRecordIdentity]) { + init( + snapshot: SyncEditSnapshot, + savedRecords: [CKRecord.ID: SyncRecordIdentity], + deletedRecords: [CKRecord.ID: SyncRecordIdentity] = [:] + ) { let guarded = savedRecords.filter { !Self.typesMergedOnPull.contains($0.value.type) } self.snapshot = snapshot self.savedRecords = guarded + self.deletedRecords = deletedRecords self.savedIdentities = Set(guarded.values) } + func withholdsDeletion(_ recordID: CKRecord.ID, tracker: SyncChangeTracker) -> Bool { + guard let identity = deletedRecords[recordID] else { return false } + return tracker.hasEdit(identity, since: snapshot) + } + func withholds(_ recordID: CKRecord.ID, tracker: SyncChangeTracker) -> Bool { guard let identity = savedRecords[recordID] else { return false } return tracker.hasEdit(identity, since: snapshot) diff --git a/TablePro/Core/Sync/SyncRecordMapper.swift b/TablePro/Core/Sync/SyncRecordMapper.swift index 90a5e113b9..d292defe49 100644 --- a/TablePro/Core/Sync/SyncRecordMapper.swift +++ b/TablePro/Core/Sync/SyncRecordMapper.swift @@ -391,7 +391,14 @@ struct SyncRecordMapper { // MARK: - Table Favorite static func toCKRecord(favoriteEntry entry: FavoriteTablesStorage.FavoriteEntry, in zone: CKRecordZone.ID) -> CKRecord { - let favoriteId = FavoriteTablesStorage.syncId(for: entry) + toCKRecord(favoriteEntry: entry, recordId: FavoriteTablesStorage.syncId(for: entry), in: zone) + } + + static func toCKRecord( + favoriteEntry entry: FavoriteTablesStorage.FavoriteEntry, + recordId favoriteId: String, + in zone: CKRecordZone.ID + ) -> CKRecord { let recordID = recordID(type: .tableFavorite, id: favoriteId, in: zone) let record = CKRecord(recordType: SyncRecordType.tableFavorite.rawValue, recordID: recordID) diff --git a/TablePro/Views/Sidebar/DatabaseTreeOutlineCoordinator.swift b/TablePro/Views/Sidebar/DatabaseTreeOutlineCoordinator.swift index 67166daef1..e66c659de5 100644 --- a/TablePro/Views/Sidebar/DatabaseTreeOutlineCoordinator.swift +++ b/TablePro/Views/Sidebar/DatabaseTreeOutlineCoordinator.swift @@ -331,7 +331,7 @@ final class DatabaseTreeOutlineCoordinator: NSObject, NSTextFieldDelegate { FavoriteTablesStorage.FavoriteEntry( connectionId: connectionId, database: ref.database, - schema: ref.table.schema, + schema: ref.favoriteSchema, name: ref.table.name ) } diff --git a/TableProTests/Core/Services/Query/CatalogEditFavoriteAdoptionTests.swift b/TableProTests/Core/Services/Query/CatalogEditFavoriteAdoptionTests.swift new file mode 100644 index 0000000000..77832147f7 --- /dev/null +++ b/TableProTests/Core/Services/Query/CatalogEditFavoriteAdoptionTests.swift @@ -0,0 +1,119 @@ +import Foundation +@testable import TablePro +import TableProSyncTransport +import Testing + +@MainActor +struct CatalogEditFavoriteAdoptionTests { + private let connection = TestFixtures.makeConnection(database: "shop") + private let metadata: SyncMetadataStorage + private let favoriteTables: FavoriteTablesStorage + private let favoriteDatabases: FavoriteDatabasesStorage + private let databaseManager: DatabaseManager + private let adoption: CatalogEditAdoption + + init() throws { + let unique = UUID().uuidString + let defaults = try #require(UserDefaults(suiteName: "com.TablePro.tests.CatalogEditFavorites.\(unique)")) + metadata = SyncMetadataStorage( + userDefaults: try #require(UserDefaults(suiteName: "com.TablePro.tests.CatalogEditFavorites.sync.\(unique)")) + ) + let tracker = SyncChangeTracker(metadataStorage: metadata) + favoriteTables = FavoriteTablesStorage(userDefaults: defaults, syncTracker: tracker) + favoriteDatabases = FavoriteDatabasesStorage(defaults: defaults, syncTracker: tracker) + let connectionStorage = ConnectionStorage( + fileURL: FileManager.default.temporaryDirectory + .appendingPathComponent("tablepro-tests") + .appendingPathComponent("catalog-edit-favorites-\(unique).json"), + userDefaults: defaults, + syncTracker: tracker, + keychain: InMemoryKeychain() + ) + databaseManager = DatabaseManager(connectionStorage: connectionStorage) + adoption = CatalogEditAdoption( + databaseManager: databaseManager, + connectionStorage: connectionStorage, + favoriteTables: favoriteTables, + favoriteDatabases: favoriteDatabases + ) + } + + private func withSession(_ body: () -> Void) { + databaseManager.injectSession(ConnectionSession(connection: connection), for: connection.id) + defer { + databaseManager.removeSession(for: connection.id) + SharedSidebarState.removeConnection(connection.id) + } + body() + } + + private func changes(of name: Notification.Name, from sender: AnyObject, during body: () -> Void) -> Int { + var notifications = 0 + let observer = NotificationCenter.default.addObserver(forName: name, object: sender, queue: nil) { _ in + notifications += 1 + } + defer { NotificationCenter.default.removeObserver(observer) } + body() + return notifications + } + + @Test("Renaming a database moves every favorite table in it, and its own favorite, one change each") + func databaseRenameMovesFavoritesInOneChange() { + let names = (0..<200).map { "table_\($0)" } + for name in names { + favoriteTables.addFavorite(name: name, schema: "public", database: "shop", connectionId: connection.id) + } + favoriteTables.addFavorite(name: "orders", schema: "public", database: "archive", connectionId: connection.id) + favoriteDatabases.setFavorite(database: "shop", environment: .production, connectionId: connection.id) + metadata.clearDirty(type: .tableFavorite) + metadata.clearDirty(type: .favoriteDatabase) + + var databaseChanges = 0 + let tableChanges = changes(of: .favoriteTablesDidChange, from: favoriteTables) { + databaseChanges = changes(of: .favoriteDatabasesDidChange, from: favoriteDatabases) { + withSession { + adoption.adoptContainerRename(.database("shop"), to: "shop_v2", connectionId: connection.id) + } + } + } + + let favorites = favoriteTables.favorites(for: connection.id) + #expect(tableChanges == 1) + #expect(databaseChanges == 1) + #expect(favorites.filter { $0.database == "shop_v2" }.count == 200) + #expect(favorites.contains { $0.database == "archive" }) + #expect(!favorites.contains { $0.database == "shop" }) + #expect(metadata.dirtyIds(for: .tableFavorite).count == 200) + #expect(metadata.tombstones(for: .tableFavorite).count == 200) + #expect(favoriteDatabases.favorites(for: connection.id).map(\.database) == ["shop_v2"]) + } + + @Test("Renaming a table moves its star, spelled by the table's own schema") + func tableRenameMovesTheStar() { + let table = TestFixtures.makeTableInfo(name: "orders", schema: "public") + let ref = DatabaseTreeTableRef(database: "shop", schema: nil, table: table) + favoriteTables.addFavorite(name: "orders", schema: "public", database: "shop", connectionId: connection.id) + + let tableChanges = changes(of: .favoriteTablesDidChange, from: favoriteTables) { + withSession { + adoption.adoptTableRename(ref, to: "orders_v2", connectionId: connection.id) + } + } + + #expect(tableChanges == 1) + #expect(favoriteTables.favorites(for: connection.id).map(\.name) == ["orders_v2"]) + } + + @Test("A star written from a table whose schema is empty follows its rename") + func emptySchemaStarFollowsTheRename() { + let table = TestFixtures.makeTableInfo(name: "orders", schema: "") + let ref = DatabaseTreeTableRef(database: "shop", schema: nil, table: table) + favoriteTables.addFavorite(name: "orders", schema: table.schema, database: "shop", connectionId: connection.id) + + withSession { + adoption.adoptTableRename(ref, to: "orders_v2", connectionId: connection.id) + } + + #expect(favoriteTables.favorites(for: connection.id).map(\.name) == ["orders_v2"]) + } +} diff --git a/TableProTests/Core/Storage/FavoriteTablesStorageTests.swift b/TableProTests/Core/Storage/FavoriteTablesStorageTests.swift index 757ab3cc14..ba373702d3 100644 --- a/TableProTests/Core/Storage/FavoriteTablesStorageTests.swift +++ b/TableProTests/Core/Storage/FavoriteTablesStorageTests.swift @@ -5,7 +5,9 @@ import Testing @MainActor struct FavoriteTablesStorageTests { - private func makeStorage() throws -> (FavoriteTablesStorage, SyncMetadataStorage) { + private static let storageKey = "com.TablePro.favoriteTables" + + private func makeFixture() throws -> (storage: FavoriteTablesStorage, defaults: UserDefaults, metadata: SyncMetadataStorage) { let favoritesSuite = "FavoriteTablesStorageTests.favorites.\(UUID().uuidString)" let syncSuite = "FavoriteTablesStorageTests.sync.\(UUID().uuidString)" let favoritesDefaults = try #require(UserDefaults(suiteName: favoritesSuite)) @@ -16,7 +18,31 @@ struct FavoriteTablesStorageTests { let metadata = SyncMetadataStorage(userDefaults: syncDefaults) let tracker = SyncChangeTracker(metadataStorage: metadata) let storage = FavoriteTablesStorage(userDefaults: favoritesDefaults, syncTracker: tracker) - return (storage, metadata) + return (storage, favoritesDefaults, metadata) + } + + private func makeStorage() throws -> (FavoriteTablesStorage, SyncMetadataStorage) { + let fixture = try makeFixture() + return (fixture.storage, fixture.metadata) + } + + private func countingChanges(of storage: FavoriteTablesStorage, during body: () -> Void) -> Int { + var notifications = 0 + let observer = NotificationCenter.default.addObserver( + forName: .favoriteTablesDidChange, object: storage, queue: nil + ) { _ in notifications += 1 } + defer { NotificationCenter.default.removeObserver(observer) } + body() + return notifications + } + + private func entry( + _ name: String, + database: String? = "shop", + schema: String? = nil, + connectionId: UUID + ) -> FavoriteTablesStorage.FavoriteEntry { + FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: database, schema: schema, name: name) } /// The only writer of a table favorite keys it on the table's own schema, so anything reading @@ -123,13 +149,16 @@ struct FavoriteTablesStorageTests { #expect(metadata.tombstones(for: .tableFavorite).contains { $0.id == id }) } - @Test("Remote apply helpers do not track local sync changes") - func withoutSyncDoesNotTrackChanges() throws { + @Test("Remote apply does not track local sync changes") + func remoteApplyDoesNotTrackChanges() throws { let (storage, metadata) = try makeStorage() let connId = UUID() let entry = FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: nil, schema: nil, name: "orders") - storage.addFavoriteWithoutSync(entry) - storage.removeFavoriteWithoutSync(entry) + let id = FavoriteTablesStorage.syncId(for: entry) + storage.applyRemote(saved: [entry], deletedIds: []) + #expect(storage.loadFavorites() == [entry]) + + storage.applyRemote(saved: [], deletedIds: [id]) #expect(storage.loadFavorites().isEmpty) #expect(metadata.dirtyIds(for: .tableFavorite).isEmpty) @@ -187,4 +216,227 @@ struct FavoriteTablesStorageTests { #expect(storage.favorites(for: connId).isEmpty) #expect(metadata.dirtyIds(for: .tableFavorite).isEmpty) } + + @Test("Moving a database's favorites to its new name writes one change for all of them") + func retargetingManyFavoritesIsOneChange() throws { + let (storage, metadata) = try makeStorage() + let connId = UUID() + let names = (0..<200).map { "table_\($0)" } + for name in names { + storage.addFavorite(name: name, schema: "public", database: "shop", connectionId: connId) + } + metadata.clearDirty(type: .tableFavorite) + + let notifications = countingChanges(of: storage) { + storage.retarget(connectionId: connId) { favorite in + entry(favorite.name, database: "shop_v2", schema: favorite.schema, connectionId: connId) + } + } + + let moved = Set(names.map { entry($0, database: "shop_v2", schema: "public", connectionId: connId) }) + let original = Set(names.map { entry($0, database: "shop", schema: "public", connectionId: connId) }) + #expect(notifications == 1) + #expect(storage.favorites(for: connId) == moved) + #expect(metadata.dirtyIds(for: .tableFavorite) == Set(moved.map(FavoriteTablesStorage.syncId(for:)))) + #expect(Set(metadata.tombstones(for: .tableFavorite).map(\.id)) == Set(original.map(FavoriteTablesStorage.syncId(for:)))) + } + + @Test("Moving one connection's favorites leaves another connection's alone") + func retargetIsScopedToItsConnection() throws { + let (storage, metadata) = try makeStorage() + let connId = UUID() + let other = UUID() + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connId) + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: other) + metadata.clearDirty(type: .tableFavorite) + + storage.retarget(connectionId: connId) { entry($0.name, database: "archive", connectionId: connId) } + + #expect(storage.favorites(for: other) == [entry("orders", connectionId: other)]) + #expect(storage.favorites(for: connId) == [entry("orders", database: "archive", connectionId: connId)]) + #expect(!metadata.tombstones(for: .tableFavorite).contains { + $0.id == FavoriteTablesStorage.syncId(for: entry("orders", connectionId: other)) + }) + } + + @Test("Renaming a table and renaming it back leaves the original to save and the detour to delete") + func renameRoundTripLeavesTheOriginalDirty() throws { + let (storage, metadata) = try makeStorage() + let connId = UUID() + let original = entry("orders", connectionId: connId) + let detour = entry("orders_old", connectionId: connId) + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connId) + metadata.clearDirty(type: .tableFavorite) + + storage.retarget(connectionId: connId) { $0 == original ? detour : $0 } + storage.retarget(connectionId: connId) { $0 == detour ? original : $0 } + + let originalId = FavoriteTablesStorage.syncId(for: original) + let detourId = FavoriteTablesStorage.syncId(for: detour) + #expect(storage.favorites(for: connId) == [original]) + #expect(metadata.dirtyIds(for: .tableFavorite) == [originalId]) + #expect(Set(metadata.tombstones(for: .tableFavorite).map(\.id)) == [originalId, detourId]) + } + + @Test("Moving a favorite onto one that already exists merges the two") + func retargetOntoAnExistingFavoriteMerges() throws { + let (storage, metadata) = try makeStorage() + let connId = UUID() + let source = entry("orders", connectionId: connId) + let target = entry("orders_v2", connectionId: connId) + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connId) + storage.addFavorite(name: "orders_v2", schema: nil, database: "shop", connectionId: connId) + metadata.clearDirty(type: .tableFavorite) + + storage.retarget(connectionId: connId) { $0 == source ? target : $0 } + + #expect(storage.favorites(for: connId) == [target]) + #expect(metadata.dirtyIds(for: .tableFavorite).isEmpty) + #expect(metadata.tombstones(for: .tableFavorite).map(\.id) == [FavoriteTablesStorage.syncId(for: source)]) + } + + @Test("A change that changes nothing writes nothing and posts nothing") + func noOpChangesPostNothing() throws { + let (storage, metadata) = try makeStorage() + let connId = UUID() + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connId) + metadata.clearDirty(type: .tableFavorite) + + let notifications = countingChanges(of: storage) { + storage.removeFavorite(name: "missing", schema: nil, database: "shop", connectionId: connId) + storage.retarget(connectionId: connId) { $0 } + storage.removeFavorites(inDatabase: "other", schema: nil, connectionId: connId) + #expect(!storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connId)) + } + + #expect(notifications == 0) + #expect(metadata.dirtyIds(for: .tableFavorite).isEmpty) + #expect(metadata.tombstones(for: .tableFavorite).isEmpty) + } + + @Test("Dropping a schema posts one change for every favorite it held") + func droppingAContainerIsOneChange() throws { + let (storage, metadata) = try makeStorage() + let connId = UUID() + for name in ["orders", "invoices", "customers"] { + storage.addFavorite(name: name, schema: "public", database: "shop", connectionId: connId) + } + + let notifications = countingChanges(of: storage) { + storage.removeFavorites(inDatabase: "shop", schema: "public", connectionId: connId) + } + + #expect(notifications == 1) + #expect(storage.favorites(for: connId).isEmpty) + #expect(metadata.tombstones(for: .tableFavorite).count == 3) + } + + @Test("Dropping a database named by an empty string removes the favorites stored without one") + func droppingAnEmptyDatabaseNameMatchesFavoritesWithoutOne() throws { + let (storage, _) = try makeStorage() + let connId = UUID() + storage.addFavorite(name: "orders", schema: nil, database: nil, connectionId: connId) + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connId) + + storage.removeFavorites(inDatabase: "", schema: nil, connectionId: connId) + + #expect(storage.favorites(for: connId) == [entry("orders", connectionId: connId)]) + } + + @Test("An empty database or schema is the same favorite as none") + func emptyContainersAreNone() { + let connId = UUID() + let empty = FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "", schema: "", name: "orders") + let none = FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: nil, schema: nil, name: "orders") + + #expect(empty == none) + #expect(empty.database == nil) + #expect(empty.schema == nil) + #expect(FavoriteTablesStorage.syncId(for: empty) == FavoriteTablesStorage.syncId(for: none)) + } + + @Test("Stored favorites with empty containers load as none, and duplicates merge into one stored entry") + func storedEmptyContainersMergeOnLoad() throws { + let (storage, defaults, _) = try makeFixture() + let connId = UUID().uuidString + let json = """ + [{"connectionId":"\(connId)","database":"","schema":"","name":"orders"}, + {"connectionId":"\(connId)","name":"orders"}] + """ + defaults.set(Data(json.utf8), forKey: Self.storageKey) + + let loaded = storage.loadFavorites() + + #expect(loaded.count == 1) + #expect(loaded.first?.database == nil) + #expect(loaded.first?.schema == nil) + let stored = try JSONDecoder().decode( + [FavoriteTablesStorage.FavoriteEntry].self, + from: try #require(defaults.data(forKey: Self.storageKey)) + ) + #expect(stored.count == 1) + } + + @Test("Re-keying at launch marks only the favorites whose id changed, under both ids, and tombstones nothing") + func migrationMarksOnlyChangedIds() throws { + let (storage, defaults, metadata) = try makeFixture() + let connId = UUID() + let piped = entry("orders", schema: "a|b", connectionId: connId) + let slashed = entry("back\\slash", connectionId: connId) + let plain = entry("orders", schema: "public", connectionId: connId) + defaults.set(try JSONEncoder().encode([piped, slashed, plain]), forKey: Self.storageKey) + + storage.migrateSyncIdentityIfNeeded() + + let expected = Set([piped, slashed].map(FavoriteTablesStorage.syncId(for:))) + .union([piped, slashed].compactMap(FavoriteTablesStorage.legacyAlias(of:))) + #expect(expected.count == 4) + #expect(metadata.dirtyIds(for: .tableFavorite) == expected) + #expect(metadata.tombstones(for: .tableFavorite).isEmpty) + #expect(storage.favorites(for: connId) == [piped, slashed, plain]) + + metadata.clearDirty(type: .tableFavorite) + storage.migrateSyncIdentityIfNeeded() + + #expect(metadata.dirtyIds(for: .tableFavorite).isEmpty) + } + + @Test("An old-build deletion that two re-keyed favorites could both answer to removes neither") + func ambiguousLegacyDeletionRemovesNothing() throws { + let (storage, defaults, metadata) = try makeFixture() + let connId = UUID() + let first = entry("t", database: "a|b", schema: "c", connectionId: connId) + let second = entry("t", database: "a", schema: "b|c", connectionId: connId) + let legacyId = try #require(FavoriteTablesStorage.legacyAlias(of: first)) + #expect(FavoriteTablesStorage.legacyAlias(of: second) == legacyId) + defaults.set(try JSONEncoder().encode([first, second]), forKey: Self.storageKey) + storage.migrateSyncIdentityIfNeeded() + metadata.clearDirty(type: .tableFavorite) + + let retired = storage.applyRemote(saved: [], deletedIds: [legacyId]) + + #expect(retired.isEmpty) + #expect(storage.favorites(for: connId) == [first, second]) + } + + @Test("Removing one of two favorites that shared an old id keeps the old record for the other") + func sharedLegacyIdOutlivesOneRemoval() throws { + let (storage, defaults, metadata) = try makeFixture() + let connId = UUID() + let first = entry("t", database: "a|b", schema: "c", connectionId: connId) + let second = entry("t", database: "a", schema: "b|c", connectionId: connId) + defaults.set(try JSONEncoder().encode([first, second]), forKey: Self.storageKey) + storage.migrateSyncIdentityIfNeeded() + + let sharedAlias = try #require(FavoriteTablesStorage.legacyAlias(of: first)) + + storage.removeFavorite(name: "t", schema: "c", database: "a|b", connectionId: connId) + #expect(metadata.tombstones(for: .tableFavorite).map(\.id) == [FavoriteTablesStorage.syncId(for: first)]) + + storage.removeFavorite(name: "t", schema: "b|c", database: "a", connectionId: connId) + #expect( + Set(metadata.tombstones(for: .tableFavorite).map(\.id)) + == [FavoriteTablesStorage.syncId(for: first), FavoriteTablesStorage.syncId(for: second), sharedAlias] + ) + } } diff --git a/TableProTests/Core/Storage/SyncDirtyMarkingTests.swift b/TableProTests/Core/Storage/SyncDirtyMarkingTests.swift index e48e15b100..aeb17eb409 100644 --- a/TableProTests/Core/Storage/SyncDirtyMarkingTests.swift +++ b/TableProTests/Core/Storage/SyncDirtyMarkingTests.swift @@ -145,7 +145,7 @@ struct SyncDirtyMarkingTests { let tableId = String(repeating: "a", count: 64) tracker.markDirty(.tableFavorite, id: tableId) - tables.removeFavoriteWithoutSync(id: tableId) + tables.applyRemote(saved: [], deletedIds: [tableId]) #expect(metadata.dirtyIds(for: .tableFavorite).isEmpty) } diff --git a/TableProTests/Core/Sync/FavoriteTableSyncIdentityTests.swift b/TableProTests/Core/Sync/FavoriteTableSyncIdentityTests.swift new file mode 100644 index 0000000000..7ef8e7e60e --- /dev/null +++ b/TableProTests/Core/Sync/FavoriteTableSyncIdentityTests.swift @@ -0,0 +1,307 @@ +import CloudKit +import Foundation +@testable import TablePro +import TableProSyncTransport +import Testing + +@MainActor +struct FavoriteTableSyncIdentityTests { + private static let zoneID = SyncTestEnvironment.zoneID + private static let storageKey = "com.TablePro.favoriteTables" + + private let environment: SyncTestEnvironment + private let connectionId = UUID() + + init() throws { + environment = try SyncTestEnvironment(label: "favorite-table-identity") + } + + private var tables: FavoriteTablesStorage { environment.favoriteTables } + private var tracker: SyncChangeTracker { environment.tracker } + + private var pipedEntry: FavoriteTablesStorage.FavoriteEntry { + FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: "shop", schema: "a|b", name: "orders") + } + + private func storeBeforeUpgrade(_ entries: [FavoriteTablesStorage.FavoriteEntry]) throws { + environment.defaults.set(try JSONEncoder().encode(entries), forKey: Self.storageKey) + } + + private func recordID(_ id: String) -> CKRecord.ID { + SyncRecordMapper.recordID(type: .tableFavorite, id: id, in: Self.zoneID) + } + + private func aliasId(of entry: FavoriteTablesStorage.FavoriteEntry) throws -> String { + try #require(FavoriteTablesStorage.legacyAlias(of: entry)) + } + + private func legacyRecord(for entry: FavoriteTablesStorage.FavoriteEntry) throws -> CKRecord { + let current = SyncRecordMapper.toCKRecord(favoriteEntry: entry, in: Self.zoneID) + let legacy = CKRecord(recordType: current.recordType, recordID: recordID(try aliasId(of: entry))) + for key in current.allKeys() { + legacy[key] = current[key] + } + return legacy + } + + private func echoEverything() -> ScriptedSyncTransport { + ScriptedSyncTransport(zoneID: Self.zoneID) { records, deletions in + PullResult(changedRecords: records, deletedRecordIDs: deletions, newToken: nil) + } + } + + @Test("A favorite re-keyed at launch survives the echo of its own push") + func migratedFavoriteSurvivesItsEcho() async throws { + try storeBeforeUpgrade([pipedEntry]) + tables.migrateSyncIdentityIfNeeded() + let transport = echoEverything() + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(tables.favorites(for: connectionId) == [pipedEntry]) + let alias = try aliasId(of: pipedEntry) + #expect( + Set(await transport.pushedRecords.map(\.recordID)) + == [recordID(FavoriteTablesStorage.syncId(for: pipedEntry)), recordID(alias)] + ) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("A favorite starred after the upgrade is also saved under its old id, for Macs on an older build") + func postUpgradeFavoriteIsSavedUnderBothIds() async throws { + tables.addFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: connectionId) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + let pushed = await transport.pushedRecords + let aliasRecordID = recordID(try aliasId(of: pipedEntry)) + let alias = try #require(pushed.first { $0.recordID == aliasRecordID }) + #expect(failure == nil) + #expect(pushed.count == 2) + #expect(try SyncRecordMapper.favoriteEntry(from: alias) == pipedEntry) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("Two favorites that share an old id are saved under their new ids alone") + func sharedOldIdIsNotPublished() async throws { + tables.addFavorite(name: "t", schema: "c", database: "a|b", connectionId: connectionId) + tables.addFavorite(name: "t", schema: "b|c", database: "a", connectionId: connectionId) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + let ids = Set(tables.favorites(for: connectionId).map { recordID(FavoriteTablesStorage.syncId(for: $0)) }) + #expect(failure == nil) + #expect(Set(await transport.pushedRecords.map(\.recordID)) == ids) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("Removing one of two favorites that shared an old id republishes that id for the one left") + func sharedOldIdIsRepublishedForTheSurvivor() async throws { + tables.addFavorite(name: "t", schema: "c", database: "a|b", connectionId: connectionId) + tables.addFavorite(name: "t", schema: "b|c", database: "a", connectionId: connectionId) + _ = await environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)).runSyncCycle() + let survivor = FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: "a", schema: "b|c", name: "t") + let alias = try aliasId(of: survivor) + + tables.removeFavorite(name: "t", schema: "c", database: "a|b", connectionId: connectionId) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + let aliasRecordID = recordID(alias) + let republished = try #require(await transport.pushedRecords.first { $0.recordID == aliasRecordID }) + #expect(failure == nil) + #expect(try SyncRecordMapper.favoriteEntry(from: republished) == survivor) + #expect(!(await transport.pushedDeletions.contains(aliasRecordID))) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("A re-keyed favorite starred again while its deletion is in flight survives the echo under both ids") + func restarOfARekeyedFavoriteDuringTheDeletionSurvives() async throws { + let storage = tables + let owner = connectionId + storage.addFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: owner) + environment.metadata.clearDirty(type: .tableFavorite) + storage.removeFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: owner) + let transport = ScriptedSyncTransport( + zoneID: Self.zoneID, + duringPush: { storage.addFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: owner) }, + echoing: { records, deletions in PullResult(changedRecords: records, deletedRecordIDs: deletions, newToken: nil) } + ) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(await transport.pushedDeletions.count == 2) + #expect(tables.favorites(for: connectionId) == [pipedEntry]) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + let alias = try aliasId(of: pipedEntry) + #expect(tracker.dirtyRecords(for: .tableFavorite) == [FavoriteTablesStorage.syncId(for: pipedEntry), alias]) + } + + @Test("A favorite starred again just before the upgrade survives the first sync after it") + func restarredBeforeUpgradeSurvives() async throws { + try storeBeforeUpgrade([pipedEntry]) + let legacyId = try aliasId(of: pipedEntry) + tracker.markDeleted(.tableFavorite, id: legacyId) + tracker.markDirty(.tableFavorite, id: legacyId) + tables.migrateSyncIdentityIfNeeded() + let transport = echoEverything() + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(tables.favorites(for: connectionId) == [pipedEntry]) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("A Mac on an older build deleting a favorite starred after the upgrade removes it too") + func legacyDeletionOfAPostUpgradeFavorite() async throws { + tables.migrateSyncIdentityIfNeeded() + tables.addFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: connectionId) + environment.metadata.clearDirty(type: .tableFavorite) + let alias = try aliasId(of: pipedEntry) + + let acknowledged = await environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + .applyPullResult(PullResult(changedRecords: [], deletedRecordIDs: [recordID(alias)], newToken: nil)) + + #expect(acknowledged) + #expect(tables.favorites(for: connectionId).isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite) == [FavoriteTablesStorage.syncId(for: pipedEntry)]) + } + + @Test("A favorite starred again while its deletion is in flight survives the echo of that deletion") + func restarDuringTheDeletionSurvivesItsEcho() async throws { + let entry = FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: "shop", schema: nil, name: "orders") + let storage = tables + storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connectionId) + environment.metadata.clearDirty(type: .tableFavorite) + storage.removeFavorite(name: "orders", schema: nil, database: "shop", connectionId: connectionId) + let transport = ScriptedSyncTransport( + zoneID: Self.zoneID, + duringPush: { storage.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: entry.connectionId) }, + echoing: { records, deletions in PullResult(changedRecords: records, deletedRecordIDs: deletions, newToken: nil) } + ) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + let id = FavoriteTablesStorage.syncId(for: entry) + #expect(failure == nil) + #expect(await transport.pushedDeletions == [recordID(id)]) + #expect(tables.favorites(for: connectionId) == [entry]) + #expect(tracker.dirtyRecords(for: .tableFavorite) == [id]) + + let next = ScriptedSyncTransport(zoneID: Self.zoneID) + _ = await environment.makeCoordinator(transport: next).runSyncCycle() + + #expect(await next.pushedRecords.map(\.recordID) == [recordID(id)]) + #expect(await next.pushedDeletions.isEmpty) + } + + @Test("A favorite saved in the same pull that deletes its connection is not kept") + func connectionDeletionOutranksAFavoriteSave() async throws { + let connection = TestFixtures.makeConnection(name: "Removed") + environment.connections.addConnection(connection) + let favorite = FavoriteTablesStorage.FavoriteEntry( + connectionId: connection.id, database: "shop", schema: nil, name: "orders" + ) + + let acknowledged = await environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + .applyPullResult(PullResult( + changedRecords: [SyncRecordMapper.toCKRecord(favoriteEntry: favorite, in: Self.zoneID)], + deletedRecordIDs: [ + SyncRecordMapper.recordID(type: .connection, id: connection.id.uuidString, in: Self.zoneID) + ], + newToken: nil + )) + + #expect(acknowledged) + #expect(environment.connections.loadConnection(id: connection.id) == nil) + #expect(tables.favorites(for: connection.id).isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + } + + @Test("A Mac on an older build deleting the favorite by its old id removes it and retires the new id") + func legacyDeletionRemovesTheFavorite() async throws { + try storeBeforeUpgrade([pipedEntry]) + tables.migrateSyncIdentityIfNeeded() + let legacyId = try aliasId(of: pipedEntry) + + let acknowledged = await environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + .applyPullResult(PullResult(changedRecords: [], deletedRecordIDs: [recordID(legacyId)], newToken: nil)) + + #expect(acknowledged) + #expect(tables.favorites(for: connectionId).isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite) == [FavoriteTablesStorage.syncId(for: pipedEntry)]) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("Removing a re-keyed favorite deletes both of its records, and their echo changes nothing") + func localRemovalDeletesBothRecords() async throws { + try storeBeforeUpgrade([pipedEntry]) + tables.migrateSyncIdentityIfNeeded() + tables.removeFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: connectionId) + let currentId = FavoriteTablesStorage.syncId(for: pipedEntry) + let legacyId = try aliasId(of: pipedEntry) + #expect(tracker.tombstonedIds(for: .tableFavorite) == [currentId, legacyId]) + let transport = echoEverything() + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(Set(await transport.pushedDeletions) == [recordID(currentId), recordID(legacyId)]) + #expect(tables.favorites(for: connectionId).isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } + + @Test("A favorite pulled from a Mac on an older build is deleted under both ids when removed here") + func pulledLegacyRecordIsRetiredOnRemoval() async throws { + let acknowledged = await environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + .applyPullResult(PullResult(changedRecords: [try legacyRecord(for: pipedEntry)], deletedRecordIDs: [], newToken: nil)) + #expect(acknowledged) + #expect(tables.favorites(for: connectionId) == [pipedEntry]) + + let legacyId = try aliasId(of: pipedEntry) + + tables.removeFavorite(name: "orders", schema: "a|b", database: "shop", connectionId: connectionId) + + #expect(tracker.tombstonedIds(for: .tableFavorite) == [FavoriteTablesStorage.syncId(for: pipedEntry), legacyId]) + } + + @Test("A pull applies all of its table favorite saves and deletions as one change") + func pullAppliesTableFavoritesAsOneChange() async throws { + let kept = FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: "shop", schema: nil, name: "kept") + let doomed = ["gone_1", "gone_2"].map { + FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: "shop", schema: nil, name: $0) + } + try storeBeforeUpgrade([kept] + doomed) + let arriving = ["new_1", "new_2", "new_3"].map { + FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, database: "shop", schema: nil, name: $0) + } + var notifications = 0 + let observer = NotificationCenter.default.addObserver( + forName: .favoriteTablesDidChange, object: tables, queue: nil + ) { _ in notifications += 1 } + defer { NotificationCenter.default.removeObserver(observer) } + + let acknowledged = await environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + .applyPullResult(PullResult( + changedRecords: arriving.map { SyncRecordMapper.toCKRecord(favoriteEntry: $0, in: Self.zoneID) }, + deletedRecordIDs: doomed.map { recordID(FavoriteTablesStorage.syncId(for: $0)) }, + newToken: nil + )) + + #expect(acknowledged) + #expect(notifications == 1) + #expect(tables.favorites(for: connectionId) == Set([kept] + arriving)) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + } +} diff --git a/TableProTests/Core/Sync/SyncCoordinatorEchoTests.swift b/TableProTests/Core/Sync/SyncCoordinatorEchoTests.swift index 43dcf3c82d..09911158b9 100644 --- a/TableProTests/Core/Sync/SyncCoordinatorEchoTests.swift +++ b/TableProTests/Core/Sync/SyncCoordinatorEchoTests.swift @@ -6,15 +6,10 @@ import Testing @MainActor struct SyncCoordinatorEchoTests { - private static let zoneID = CKRecordZone.ID( - zoneName: CloudKitSyncEngine.zoneName, - ownerName: CKCurrentUserDefaultName - ) + private static let zoneID = SyncTestEnvironment.zoneID - private let unique = UUID().uuidString - private let keychain = InMemoryKeychain() + private let environment: SyncTestEnvironment private let directory: URL - private let defaults: UserDefaults private let metadata: SyncMetadataStorage private let tracker: SyncChangeTracker private let recordCache: SyncRecordCache @@ -22,51 +17,19 @@ struct SyncCoordinatorEchoTests { private let groups: GroupStorage private let tags: TagStorage private let favoriteDatabases: FavoriteDatabasesStorage - private let columnLayouts: FileColumnLayoutPersister private let favorites: SQLFavoriteManager init() throws { - directory = FileManager.default.temporaryDirectory - .appendingPathComponent("tablepro-tests") - .appendingPathComponent("sync-echo-\(unique)", isDirectory: true) - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) - defaults = try #require(UserDefaults(suiteName: "com.TablePro.tests.SyncEcho.\(unique)")) - metadata = SyncMetadataStorage( - userDefaults: try #require(UserDefaults(suiteName: "com.TablePro.tests.SyncEcho.sync.\(unique)")) - ) - tracker = SyncChangeTracker(metadataStorage: metadata) - recordCache = SyncRecordCache( - directory: directory.appendingPathComponent("SyncRecordCache", isDirectory: true), - defaults: nil - ) - let connectionStore = ConnectionStorage( - fileURL: directory.appendingPathComponent("connections.json"), - userDefaults: defaults, - syncTracker: tracker, - keychain: keychain, - integrity: ConnectionStoreIntegrity(keySource: StoredIntegrityKeySource(store: keychain)) - ) - connections = connectionStore - groups = GroupStorage( - userDefaults: defaults, - syncTracker: tracker, - connectionStorage: connectionStore, - appEvents: AppEvents() - ) - tags = TagStorage(userDefaults: defaults, syncTracker: tracker, appEvents: AppEvents()) - favoriteDatabases = FavoriteDatabasesStorage(defaults: defaults, syncTracker: tracker) - columnLayouts = FileColumnLayoutPersister( - storageDirectory: directory.appendingPathComponent("ColumnLayout", isDirectory: true), - defaults: defaults, - syncTracker: tracker - ) - favorites = SQLFavoriteManager( - storage: SQLFavoriteStorage( - databaseURL: directory.appendingPathComponent("sql_favorites.db"), - removeDatabaseOnDeinit: true - ), - syncTracker: tracker - ) + environment = try SyncTestEnvironment(label: "sync-echo") + directory = environment.directory + metadata = environment.metadata + tracker = environment.tracker + recordCache = environment.recordCache + connections = environment.connections + groups = environment.groups + tags = environment.tags + favoriteDatabases = environment.favoriteDatabases + favorites = environment.favorites } @Test("A tag edited while its push is in flight keeps the edit, and a sibling's remote change still lands") @@ -229,7 +192,7 @@ struct SyncCoordinatorEchoTests { @Test("A tag and a group deleted elsewhere stay here while Groups and Tags is off") func remoteDeletionsWithheldWhileCategoryOff() async throws { - AppSettingsStorage(userDefaults: defaults).saveSync( + AppSettingsStorage(userDefaults: environment.defaults).saveSync( SyncSettings(enabled: true, syncConnections: true, syncGroupsAndTags: false, syncSettings: true) ) let tag = ConnectionTag(name: "staging") @@ -251,7 +214,7 @@ struct SyncCoordinatorEchoTests { @Test("A tag changed elsewhere keeps its local name while Groups and Tags is off") func remoteChangeWithheldWhileCategoryOff() async throws { - AppSettingsStorage(userDefaults: defaults).saveSync( + AppSettingsStorage(userDefaults: environment.defaults).saveSync( SyncSettings(enabled: true, syncConnections: true, syncGroupsAndTags: false, syncSettings: true) ) let tag = ConnectionTag(name: "staging") @@ -609,57 +572,7 @@ struct SyncCoordinatorEchoTests { transport: ScriptedSyncTransport, favorites: SQLFavoriteManager? = nil ) -> SyncCoordinator { - let live = AppServices.live - let services = AppServices( - appEvents: AppEvents(), - appSettings: live.appSettings, - appSettingsStorage: AppSettingsStorage(userDefaults: defaults), - connectionStorage: connections, - databaseManager: live.databaseManager, - pluginManager: live.pluginManager, - schemaService: live.schemaService, - schemaRefreshService: live.schemaRefreshService, - schemaProviderRegistry: live.schemaProviderRegistry, - catalogChangeService: live.catalogChangeService, - sqlFavoriteManager: favorites ?? self.favorites, - favoriteTablesStorage: FavoriteTablesStorage(userDefaults: defaults, syncTracker: tracker), - favoriteDatabasesStorage: favoriteDatabases, - aiChatStorage: live.aiChatStorage, - aiKeyStorage: live.aiKeyStorage, - aiAccessApprovals: live.aiAccessApprovals, - groupStorage: groups, - tagStorage: tags, - sshProfileStorage: SSHProfileStorage( - userDefaults: defaults, - keychain: keychain, - syncTracker: tracker, - connectionStorage: connections - ), - credentialProfileStorage: CredentialProfileStorage( - fileURL: directory.appendingPathComponent("credentialProfiles.json"), - keychain: keychain, - syncTracker: tracker, - connectionStorage: connections, - integrity: ConnectionStoreIntegrity(keySource: StoredIntegrityKeySource(store: keychain)) - ), - licenseManager: live.licenseManager, - syncMetadataStorage: metadata, - favoritesExpansionState: live.favoritesExpansionState, - linkedFolderWatcher: live.linkedFolderWatcher, - queryHistoryManager: live.queryHistoryManager, - dateFormattingService: live.dateFormattingService, - copilotService: live.copilotService, - mcpServerManager: live.mcpServerManager, - syncTracker: tracker, - themeEngine: live.themeEngine, - welcomeRouter: live.welcomeRouter - ) - return SyncCoordinator( - services: services, - recordCache: recordCache, - transport: transport, - columnLayouts: columnLayouts - ) + environment.makeCoordinator(transport: transport, favorites: favorites) } } @@ -667,67 +580,3 @@ struct SyncCoordinatorEchoTests { private final class CoordinatorBox { var coordinator: SyncCoordinator? } - -private actor ScriptedSyncTransport: SyncTransport { - let currentZoneID: CKRecordZone.ID - private let rejectedRecordIDs: Set - private let interruption: (any Error)? - private let duringPush: @MainActor @Sendable () async -> Void - private let pulled: @Sendable ([CKRecord]) -> PullResult - private(set) var pushedRecords: [CKRecord] = [] - private(set) var pullCount = 0 - - init( - zoneID: CKRecordZone.ID, - rejecting rejectedRecordIDs: Set = [], - interruption: (any Error)? = nil, - duringPush: @escaping @MainActor @Sendable () async -> Void = {}, - pulled: @escaping @Sendable ([CKRecord]) -> PullResult = { _ in - PullResult(changedRecords: [], deletedRecordIDs: [], newToken: nil) - } - ) { - self.currentZoneID = zoneID - self.rejectedRecordIDs = rejectedRecordIDs - self.interruption = interruption - self.duringPush = duringPush - self.pulled = pulled - } - - func accountStatus() async throws -> CKAccountStatus { - .available - } - - func currentAccountId() async throws -> String { - "tests" - } - - func ensureZoneExists() async throws {} - - func push(records: [CKRecord], deletions: [CKRecord.ID]) async throws -> PushOutcome { - pushedRecords.append(contentsOf: records) - await duringPush() - var outcome = PushOutcome() - for record in records { - guard rejectedRecordIDs.contains(record.recordID) else { - outcome.recordSave(record) - continue - } - outcome.recordFailure( - SyncItemFailure(code: .serverRejectedRequest, serverRecord: nil, clientRecord: record, message: "Rejected"), - for: record.recordID - ) - } - for recordID in deletions { - outcome.recordDeletion(recordID) - } - if let interruption { - throw SyncPushInterruption(completed: outcome, cause: interruption) - } - return outcome - } - - func pull(since token: CKServerChangeToken?) async throws -> PullResult { - pullCount += 1 - return pulled(pushedRecords) - } -} diff --git a/TableProTests/Core/Sync/SyncPushSettlementTests.swift b/TableProTests/Core/Sync/SyncPushSettlementTests.swift new file mode 100644 index 0000000000..fa7155968c --- /dev/null +++ b/TableProTests/Core/Sync/SyncPushSettlementTests.swift @@ -0,0 +1,171 @@ +import CloudKit +import Foundation +@testable import TablePro +import TableProSyncTransport +import Testing + +@MainActor +struct SyncPushSettlementTests { + private static let zoneID = SyncTestEnvironment.zoneID + + private let environment: SyncTestEnvironment + + init() throws { + environment = try SyncTestEnvironment(label: "sync-push-settlement") + } + + private var tracker: SyncChangeTracker { environment.tracker } + private var metadata: SyncMetadataStorage { environment.metadata } + + private func tableFavoriteRecordID(_ entry: FavoriteTablesStorage.FavoriteEntry) -> CKRecord.ID { + SyncRecordMapper.recordID(type: .tableFavorite, id: FavoriteTablesStorage.syncId(for: entry), in: Self.zoneID) + } + + @Test("A table starred again before the push is sent as a save alone, and its tombstone clears") + func restarredFavoriteIsSavedNotDeleted() async throws { + let connectionId = UUID() + let tables = environment.favoriteTables + tables.toggle(name: "orders", schema: "public", database: "shop", connectionId: connectionId) + tables.toggle(name: "orders", schema: "public", database: "shop", connectionId: connectionId) + tables.toggle(name: "orders", schema: "public", database: "shop", connectionId: connectionId) + let entry = FavoriteTablesStorage.FavoriteEntry( + connectionId: connectionId, database: "shop", schema: "public", name: "orders" + ) + let id = FavoriteTablesStorage.syncId(for: entry) + #expect(tracker.dirtyRecords(for: .tableFavorite) == [id]) + #expect(tracker.tombstonedIds(for: .tableFavorite) == [id]) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(await transport.pushedRecords.map(\.recordID) == [tableFavoriteRecordID(entry)]) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + } + + @Test("A record marked dirty and then tombstoned while it still exists is sent as a save alone") + func dirtyThenTombstonedExistingRecordIsSaved() async throws { + let tag = ConnectionTag(name: "staging") + try environment.tags.addTag(tag) + metadata.addTombstone(tag.id.uuidString, type: .tag) + let recordID = SyncRecordMapper.recordID(type: .tag, id: tag.id.uuidString, in: Self.zoneID) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(await transport.pushedRecords.map(\.recordID).contains(recordID)) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.tombstonedIds(for: .tag).isEmpty) + #expect(environment.tags.tag(for: tag.id) != nil) + } + + @Test("A record both dirty and tombstoned that no longer exists is sent as a deletion alone") + func dirtyAndTombstonedAbsentRecordIsDeleted() async throws { + let absent = UUID().uuidString + tracker.markDeleted(.tag, id: absent) + tracker.markDirty(.tag, id: absent) + let recordID = SyncRecordMapper.recordID(type: .tag, id: absent, in: Self.zoneID) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(await transport.pushedRecords.isEmpty) + #expect(await transport.pushedDeletions == [recordID]) + #expect(tracker.dirtyRecords(for: .tag).isEmpty) + #expect(tracker.tombstonedIds(for: .tag).isEmpty) + } + + @Test("A table favorite unstarred again while its save is in flight keeps its tombstone for the next push") + func deletionDuringTheSaveKeepsTheTombstone() async throws { + let connectionId = UUID() + let tables = environment.favoriteTables + tables.toggle(name: "orders", schema: nil, database: "shop", connectionId: connectionId) + tables.toggle(name: "orders", schema: nil, database: "shop", connectionId: connectionId) + tables.toggle(name: "orders", schema: nil, database: "shop", connectionId: connectionId) + let entry = FavoriteTablesStorage.FavoriteEntry( + connectionId: connectionId, database: "shop", schema: nil, name: "orders" + ) + let transport = ScriptedSyncTransport( + zoneID: Self.zoneID, + duringPush: { tables.toggle(name: "orders", schema: nil, database: "shop", connectionId: connectionId) } + ) + + _ = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(tables.favorites(for: connectionId).isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite) == [FavoriteTablesStorage.syncId(for: entry)]) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + + let next = ScriptedSyncTransport(zoneID: Self.zoneID) + _ = await environment.makeCoordinator(transport: next).runSyncCycle() + + #expect(await next.pushedDeletions == [tableFavoriteRecordID(entry)]) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + } + + @Test("A deletion withheld while its store cannot be read keeps both marks") + func unreadableStoreWithholdsTheConflictedDeletion() async throws { + let url = environment.directory.appendingPathComponent("not-a-database.db") + try Data(repeating: 0x2A, count: 4_096).write(to: url) + let broken = SQLFavoriteManager( + storage: SQLFavoriteStorage(databaseURL: url, removeDatabaseOnDeinit: true), + syncTracker: tracker + ) + let id = UUID().uuidString + tracker.markDeleted(.favorite, id: id) + tracker.markDirty(.favorite, id: id) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + _ = await environment.makeCoordinator(transport: transport, favorites: broken).runSyncCycle() + + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.dirtyRecords(for: .favorite).contains(id)) + #expect(tracker.tombstonedIds(for: .favorite).contains(id)) + } + + @Test("A deletion of a record iCloud never had clears its tombstone and fails nothing") + func missingDeletionSettles() async throws { + let absent = UUID().uuidString + tracker.markDeleted(.tag, id: absent) + let recordID = SyncRecordMapper.recordID(type: .tag, id: absent, in: Self.zoneID) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID, missing: [recordID]) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(await transport.pushedDeletions == [recordID]) + #expect(tracker.tombstonedIds(for: .tag).isEmpty) + } + + @Test("A database renamed and renamed back before a push saves the original and deletes only the detour") + func databaseFavoriteRoundTripSavesTheOriginal() async throws { + let connectionId = UUID() + let databases = environment.favoriteDatabases + databases.setFavorite(database: "shop", environment: .production, connectionId: connectionId) + metadata.clearDirty(type: .favoriteDatabase) + databases.rename(database: "shop", to: "shop_v2", connectionId: connectionId) + databases.rename(database: "shop_v2", to: "shop", connectionId: connectionId) + let original = FavoriteDatabaseEntry(connectionId: connectionId, database: "shop", environment: .production) + let detour = FavoriteDatabaseEntry(connectionId: connectionId, database: "shop_v2", environment: .production) + let originalID = SyncRecordMapper.recordID( + type: .favoriteDatabase, id: FavoriteDatabasesStorage.syncId(for: original), in: Self.zoneID + ) + let detourID = SyncRecordMapper.recordID( + type: .favoriteDatabase, id: FavoriteDatabasesStorage.syncId(for: detour), in: Self.zoneID + ) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await environment.makeCoordinator(transport: transport).runSyncCycle() + + #expect(failure == nil) + #expect(databases.favorites(for: connectionId) == [original]) + #expect(await transport.pushedRecords.map(\.recordID) == [originalID]) + #expect(await transport.pushedDeletions == [detourID]) + #expect(tracker.dirtyRecords(for: .favoriteDatabase).isEmpty) + #expect(tracker.tombstonedIds(for: .favoriteDatabase).isEmpty) + } +} diff --git a/TableProTests/Core/Sync/SyncRecordMapperFavoriteTableTests.swift b/TableProTests/Core/Sync/SyncRecordMapperFavoriteTableTests.swift index 52977b99e3..7fcedb4e02 100644 --- a/TableProTests/Core/Sync/SyncRecordMapperFavoriteTableTests.swift +++ b/TableProTests/Core/Sync/SyncRecordMapperFavoriteTableTests.swift @@ -1,8 +1,8 @@ import CloudKit import Foundation @testable import TablePro -import Testing import TableProSyncTransport +import Testing struct SyncRecordMapperFavoriteTableTests { private let zoneID = CKRecordZone.ID(zoneName: "TestZone", ownerName: CKCurrentUserDefaultName) @@ -65,4 +65,81 @@ struct SyncRecordMapperFavoriteTableTests { ) #expect(FavoriteTablesStorage.syncId(for: entryA) != FavoriteTablesStorage.syncId(for: entryB)) } + + @Test("A favorite whose names hold no separator keeps the sync id every earlier build gave it") + func plainNamesKeepTheirSyncId() throws { + let entry = FavoriteTablesStorage.FavoriteEntry( + connectionId: try #require(UUID(uuidString: "00000000-0000-0000-0000-000000000001")), + database: "shop", + schema: "public", + name: "users" + ) + + let expected = "d9ddf33921f51e2b0938b408b83e7de6f54d5338d28daba48a562f6ac74ba9d2" + #expect(FavoriteTablesStorage.syncId(for: entry) == expected) + #expect(FavoriteTablesStorage.legacyAlias(of: entry) == nil) + } + + @Test("Favorites whose names differ only in where a vertical bar falls get their own records") + func separatorInNamesKeepsIdsApart() { + let connId = UUID() + let pairs: [(FavoriteTablesStorage.FavoriteEntry, FavoriteTablesStorage.FavoriteEntry)] = [ + ( + FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a|b", schema: "c", name: "t"), + FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a", schema: "b|c", name: "t") + ), + ( + FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a", schema: "b", name: "c|t"), + FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a", schema: "b|c", name: "t") + ), + ( + FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a|", schema: nil, name: "b"), + FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a", schema: nil, name: "|b") + ) + ] + + for (first, second) in pairs { + #expect(FavoriteTablesStorage.legacyAlias(of: first) != nil) + #expect(FavoriteTablesStorage.legacyAlias(of: first) == FavoriteTablesStorage.legacyAlias(of: second)) + #expect(FavoriteTablesStorage.syncId(for: first) != FavoriteTablesStorage.syncId(for: second)) + } + } + + @Test("A re-keyed favorite never lands on the record another favorite used before the re-key") + func escapedIdsStayOutOfTheLegacyNamespace() { + let connId = UUID() + let piped = FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a|b", schema: "c", name: "t") + let slashed = FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "a\\", schema: "b", name: "c|t") + let unseparated = IdentityPath.joined([connId.uuidString, "a|b", "c", "t"], separator: "|").sha256 + + #expect(unseparated == FavoriteTablesStorage.legacyAlias(of: slashed)) + #expect(FavoriteTablesStorage.syncId(for: piped) != FavoriteTablesStorage.legacyAlias(of: slashed)) + #expect(FavoriteTablesStorage.syncId(for: slashed) != FavoriteTablesStorage.legacyAlias(of: piped)) + } + + @Test("A favorite with a vertical bar in its name round trips under its own record name") + func separatorNameRoundTrips() throws { + let entry = FavoriteTablesStorage.FavoriteEntry( + connectionId: UUID(), database: "shop", schema: "a|b", name: "back\\slash" + ) + let record = SyncRecordMapper.toCKRecord(favoriteEntry: entry, in: zoneID) + + #expect(record.recordID.recordName == "FavoriteTable_\(FavoriteTablesStorage.syncId(for: entry))") + #expect(try SyncRecordMapper.favoriteEntry(from: record) == entry) + } + + @Test("A record carrying an empty schema decodes as a favorite with none") + func emptySchemaDecodesAsNone() throws { + let connId = UUID() + let record = SyncRecordMapper.toCKRecord( + favoriteEntry: FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "shop", schema: nil, name: "users"), + in: zoneID + ) + record["schema"] = "" + + let decoded = try SyncRecordMapper.favoriteEntry(from: record) + + #expect(decoded.schema == nil) + #expect(decoded == FavoriteTablesStorage.FavoriteEntry(connectionId: connId, database: "shop", schema: nil, name: "users")) + } } diff --git a/TableProTests/Helpers/SyncTestEnvironment.swift b/TableProTests/Helpers/SyncTestEnvironment.swift new file mode 100644 index 0000000000..292d4ddb64 --- /dev/null +++ b/TableProTests/Helpers/SyncTestEnvironment.swift @@ -0,0 +1,224 @@ +import CloudKit +import Foundation +@testable import TablePro +import TableProSyncTransport +import Testing + +@MainActor +final class SyncTestEnvironment { + static let zoneID = CKRecordZone.ID( + zoneName: CloudKitSyncEngine.zoneName, + ownerName: CKCurrentUserDefaultName + ) + + let keychain = InMemoryKeychain() + let directory: URL + let defaults: UserDefaults + let metadata: SyncMetadataStorage + let tracker: SyncChangeTracker + let recordCache: SyncRecordCache + let connections: ConnectionStorage + let groups: GroupStorage + let tags: TagStorage + let favoriteTables: FavoriteTablesStorage + let favoriteDatabases: FavoriteDatabasesStorage + let columnLayouts: FileColumnLayoutPersister + let favorites: SQLFavoriteManager + + init(label: String) throws { + let unique = UUID().uuidString + directory = FileManager.default.temporaryDirectory + .appendingPathComponent("tablepro-tests") + .appendingPathComponent("\(label)-\(unique)", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defaults = try #require(UserDefaults(suiteName: "com.TablePro.tests.\(label).\(unique)")) + metadata = SyncMetadataStorage( + userDefaults: try #require(UserDefaults(suiteName: "com.TablePro.tests.\(label).sync.\(unique)")) + ) + tracker = SyncChangeTracker(metadataStorage: metadata) + recordCache = SyncRecordCache( + directory: directory.appendingPathComponent("SyncRecordCache", isDirectory: true), + defaults: nil + ) + let connectionStore = ConnectionStorage( + fileURL: directory.appendingPathComponent("connections.json"), + userDefaults: defaults, + syncTracker: tracker, + keychain: keychain, + integrity: ConnectionStoreIntegrity(keySource: StoredIntegrityKeySource(store: keychain)) + ) + connections = connectionStore + groups = GroupStorage( + userDefaults: defaults, + syncTracker: tracker, + connectionStorage: connectionStore, + appEvents: AppEvents() + ) + tags = TagStorage(userDefaults: defaults, syncTracker: tracker, appEvents: AppEvents()) + favoriteTables = FavoriteTablesStorage(userDefaults: defaults, syncTracker: tracker) + favoriteDatabases = FavoriteDatabasesStorage(defaults: defaults, syncTracker: tracker) + columnLayouts = FileColumnLayoutPersister( + storageDirectory: directory.appendingPathComponent("ColumnLayout", isDirectory: true), + defaults: defaults, + syncTracker: tracker + ) + favorites = SQLFavoriteManager( + storage: SQLFavoriteStorage( + databaseURL: directory.appendingPathComponent("sql_favorites.db"), + removeDatabaseOnDeinit: true + ), + syncTracker: tracker + ) + } + + func makeCoordinator( + transport: ScriptedSyncTransport, + favorites: SQLFavoriteManager? = nil + ) -> SyncCoordinator { + let live = AppServices.live + let services = AppServices( + appEvents: AppEvents(), + appSettings: live.appSettings, + appSettingsStorage: AppSettingsStorage(userDefaults: defaults), + connectionStorage: connections, + databaseManager: live.databaseManager, + pluginManager: live.pluginManager, + schemaService: live.schemaService, + schemaRefreshService: live.schemaRefreshService, + schemaProviderRegistry: live.schemaProviderRegistry, + catalogChangeService: live.catalogChangeService, + sqlFavoriteManager: favorites ?? self.favorites, + favoriteTablesStorage: favoriteTables, + favoriteDatabasesStorage: favoriteDatabases, + aiChatStorage: live.aiChatStorage, + aiKeyStorage: live.aiKeyStorage, + aiAccessApprovals: live.aiAccessApprovals, + groupStorage: groups, + tagStorage: tags, + sshProfileStorage: SSHProfileStorage( + userDefaults: defaults, + keychain: keychain, + syncTracker: tracker, + connectionStorage: self.connections + ), + credentialProfileStorage: CredentialProfileStorage( + fileURL: directory.appendingPathComponent("credentialProfiles.json"), + keychain: keychain, + syncTracker: tracker, + connectionStorage: self.connections, + integrity: ConnectionStoreIntegrity(keySource: StoredIntegrityKeySource(store: keychain)) + ), + licenseManager: live.licenseManager, + syncMetadataStorage: metadata, + favoritesExpansionState: live.favoritesExpansionState, + linkedFolderWatcher: live.linkedFolderWatcher, + queryHistoryManager: live.queryHistoryManager, + dateFormattingService: live.dateFormattingService, + copilotService: live.copilotService, + mcpServerManager: live.mcpServerManager, + syncTracker: tracker, + themeEngine: live.themeEngine, + welcomeRouter: live.welcomeRouter + ) + return SyncCoordinator( + services: services, + recordCache: recordCache, + transport: transport, + columnLayouts: self.columnLayouts + ) + } +} + +actor ScriptedSyncTransport: SyncTransport { + let currentZoneID: CKRecordZone.ID + private let rejectedRecordIDs: Set + private let missingRecordIDs: Set + private let interruption: (any Error)? + private let duringPush: @MainActor @Sendable () async -> Void + private let pulled: @Sendable (_ records: [CKRecord], _ deletions: [CKRecord.ID]) -> PullResult + private(set) var pushedRecords: [CKRecord] = [] + private(set) var pushedDeletions: [CKRecord.ID] = [] + private(set) var pullCount = 0 + + init( + zoneID: CKRecordZone.ID, + rejecting rejectedRecordIDs: Set = [], + missing missingRecordIDs: Set = [], + interruption: (any Error)? = nil, + duringPush: @escaping @MainActor @Sendable () async -> Void = {}, + pulled: @escaping @Sendable ([CKRecord]) -> PullResult = { _ in + PullResult(changedRecords: [], deletedRecordIDs: [], newToken: nil) + } + ) { + self.init( + zoneID: zoneID, + rejecting: rejectedRecordIDs, + missing: missingRecordIDs, + interruption: interruption, + duringPush: duringPush, + echoing: { records, _ in pulled(records) } + ) + } + + init( + zoneID: CKRecordZone.ID, + rejecting rejectedRecordIDs: Set = [], + missing missingRecordIDs: Set = [], + interruption: (any Error)? = nil, + duringPush: @escaping @MainActor @Sendable () async -> Void = {}, + echoing pulled: @escaping @Sendable (_ records: [CKRecord], _ deletions: [CKRecord.ID]) -> PullResult + ) { + self.currentZoneID = zoneID + self.rejectedRecordIDs = rejectedRecordIDs + self.missingRecordIDs = missingRecordIDs + self.interruption = interruption + self.duringPush = duringPush + self.pulled = pulled + } + + func accountStatus() async throws -> CKAccountStatus { + .available + } + + func currentAccountId() async throws -> String { + "tests" + } + + func ensureZoneExists() async throws {} + + func push(records: [CKRecord], deletions: [CKRecord.ID]) async throws -> PushOutcome { + pushedRecords.append(contentsOf: records) + pushedDeletions.append(contentsOf: deletions) + await duringPush() + var outcome = PushOutcome() + for record in records { + guard rejectedRecordIDs.contains(record.recordID) else { + outcome.recordSave(record) + continue + } + outcome.recordFailure( + SyncItemFailure(code: .serverRejectedRequest, serverRecord: nil, clientRecord: record, message: "Rejected"), + for: record.recordID + ) + } + for recordID in deletions { + guard missingRecordIDs.contains(recordID) else { + outcome.recordDeletion(recordID) + continue + } + outcome.recordFailure( + SyncItemFailure(code: .unknownItem, serverRecord: nil, clientRecord: nil, message: "Record not found"), + for: recordID + ) + } + if let interruption { + throw SyncPushInterruption(completed: outcome, cause: interruption) + } + return outcome + } + + func pull(since token: CKServerChangeToken?) async throws -> PullResult { + pullCount += 1 + return pulled(pushedRecords, pushedDeletions) + } +} diff --git a/docs/features/table-operations.mdx b/docs/features/table-operations.mdx index 362abc5598..6722d11d0f 100644 --- a/docs/features/table-operations.mdx +++ b/docs/features/table-operations.mdx @@ -35,6 +35,8 @@ Renaming happens in the row itself. Choose **Rename** from a table's right-click The statement runs at once instead of joining the drop and truncate queue, so **Preview SQL** never shows it. Everything bound to the table follows the new name: open tabs keep their rows, filters, sort and column widths, a favourite stays a favourite, and the Recent entry holds its place. A drop or truncate already queued against that table comes back out of the queue. +A rename or drop typed as SQL in a query tab refreshes the sidebar but leaves the star, saved filters and column layout on the old name, so a table created later under that name comes back starred with the old filters. Use **Rename** and **Delete** from the sidebar to carry them along or clear them. + A sidebar table row with its label replaced by an editable text field A sidebar table row with its label replaced by an editable text field From 39936c23181b5f9509afa18b9f14cfb7a86424bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Wed, 30 Sep 2026 17:35:46 +0700 Subject: [PATCH 2/2] fix(sync): keep Local only connections' favorites, saved queries and layouts off iCloud (#3210) --- CHANGELOG.md | 2 + .../SyncMetadataStorage.swift | 55 +- .../SyncMetadataStorageTests.swift | 73 ++- .../Core/Storage/ColumnLayoutPersister.swift | 102 +-- .../Core/Storage/ConnectionLocalState.swift | 48 +- TablePro/Core/Storage/ConnectionStorage.swift | 33 +- .../Storage/FavoriteDatabasesStorage.swift | 17 +- .../Core/Storage/FavoriteTablesStorage.swift | 27 +- .../Core/Storage/SQLFavoriteManager.swift | 42 +- .../Storage/SQLFavoriteStorage+Deletion.swift | 44 ++ .../Core/Storage/SQLFavoriteStorage.swift | 58 +- .../SyncCoordinator+PushCollection.swift | 266 ++++---- .../SyncCoordinator+RemoteDeletions.swift | 20 +- .../Extensions/SyncCoordinator+Scope.swift | 22 + TablePro/Core/Sync/SyncBoundary.swift | 60 ++ TablePro/Core/Sync/SyncChangeTracker.swift | 43 +- TablePro/Core/Sync/SyncCoordinator.swift | 21 +- .../Connection/DatabaseConnection.swift | 4 + .../FavoriteDatabasesStorageTests.swift | 4 +- .../Storage/SQLFavoriteStorageTests.swift | 2 +- .../Storage/SQLFavoriteVersionTests.swift | 4 +- .../Core/Sync/SyncBoundaryTests.swift | 111 ++++ .../Sync/SyncLocalOnlyDependentsTests.swift | 594 ++++++++++++++++++ .../Core/Sync/SyncPendingDeletionsTests.swift | 16 + docs/features/icloud-sync.mdx | 4 +- 25 files changed, 1394 insertions(+), 278 deletions(-) create mode 100644 TablePro/Core/Storage/SQLFavoriteStorage+Deletion.swift create mode 100644 TablePro/Core/Sync/Extensions/SyncCoordinator+Scope.swift create mode 100644 TablePro/Core/Sync/SyncBoundary.swift create mode 100644 TableProTests/Core/Sync/SyncBoundaryTests.swift create mode 100644 TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 29b0b8a0eb..3ec8532529 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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. ## [0.76.1] - 2026-09-29 diff --git a/Packages/TableProCore/Sources/TableProSyncTransport/SyncMetadataStorage.swift b/Packages/TableProCore/Sources/TableProSyncTransport/SyncMetadataStorage.swift index d615f5f17a..698c525efb 100644 --- a/Packages/TableProCore/Sources/TableProSyncTransport/SyncMetadataStorage.swift +++ b/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) } public func removeTombstone(_ id: String, type: SyncRecordType) { @@ -125,17 +129,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 { + Set((userDefaults.stringArray(forKey: key("ownersKeptOffSync")) ?? []).compactMap(UUID.init(uuidString:))) + } + + public func keepOffSync(owners: Set) { + guard !owners.isEmpty else { return } + saveOwnersKeptOffSync(ownersKeptOffSync().union(owners)) + } + + public func releaseOwnersKeptOffSync(_ owners: Set) { + guard !owners.isEmpty else { return } + saveOwnersKeptOffSync(ownersKeptOffSync().subtracting(owners)) + } + + private func saveOwnersKeptOffSync(_ owners: Set) { + 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? { diff --git a/Packages/TableProCore/Tests/TableProSyncTests/SyncMetadataStorageTests.swift b/Packages/TableProCore/Tests/TableProSyncTests/SyncMetadataStorageTests.swift index eaf9a0d9cd..abfaf9dea3 100644 --- a/Packages/TableProCore/Tests/TableProSyncTests/SyncMetadataStorageTests.swift +++ b/Packages/TableProCore/Tests/TableProSyncTests/SyncMetadataStorageTests.swift @@ -68,11 +68,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() diff --git a/TablePro/Core/Storage/ColumnLayoutPersister.swift b/TablePro/Core/Storage/ColumnLayoutPersister.swift index 97fa333e5a..e0523c631f 100644 --- a/TablePro/Core/Storage/ColumnLayoutPersister.swift +++ b/TablePro/Core/Storage/ColumnLayoutPersister.swift @@ -27,7 +27,7 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin var hiddenColumns: [String]? } - static let syncCategoryPrefix = "columnLayout." + nonisolated static let syncCategoryPrefix = "columnLayout." private let storageDirectory: URL private let defaults: UserDefaults @@ -141,7 +141,7 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin cache[oldScope.connectionId] = entries writeEntries(entries, for: oldScope.connectionId) syncTracker.markDirty(.settings, id: Self.syncCategory(for: newKey)) - syncTracker.markDeleted(.settings, id: Self.syncCategory(for: oldKey)) + syncTracker.markDeleted(.settings, ids: [Self.syncCategory(for: oldKey)], owner: oldScope.connectionId) } /// Moves every table's saved layout from one container to another. Same prefix rewrite as the @@ -173,7 +173,7 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin writeEntries(entries, for: connectionId) for key in moving { syncTracker.markDirty(.settings, id: Self.syncCategory(for: newPrefix + key.dropFirst(oldPrefix.count))) - syncTracker.markDeleted(.settings, id: Self.syncCategory(for: key)) + syncTracker.markDeleted(.settings, ids: [Self.syncCategory(for: key)], owner: connectionId) } } @@ -199,20 +199,14 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin entries.removeValue(forKey: key) } - if entries.isEmpty { - cache[connectionId] = [:] - removeFile(for: connectionId) - } else { - cache[connectionId] = entries - writeEntries(entries, for: connectionId) - } - syncTracker.markDeleted(.settings, ids: dropping.map(Self.syncCategory(for:))) + guard store(entries, for: connectionId) else { return } + syncTracker.markDeleted(.settings, ids: dropping.map(Self.syncCategory(for:)), owner: connectionId) } func purgeConnections(_ connectionIds: Set, leavesTombstones: Bool) { - var categories: [String] = [] + var categoriesByConnection: [UUID: Set] = [:] for connectionId in connectionIds { - categories += loadEntries(for: connectionId).keys.map(Self.syncCategory(for:)) + categoriesByConnection[connectionId] = Set(loadEntries(for: connectionId).keys.map(Self.syncCategory(for:))) cache[connectionId] = [:] removeFile(for: connectionId) } @@ -220,9 +214,9 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin /// own deletion back at it, but leaving the ids dirty means the next push looks for entries /// that are gone and never drains them. if leavesTombstones { - syncTracker.markDeleted(.settings, ids: categories) + syncTracker.markDeleted(.settings, idsByOwner: categoriesByConnection) } else { - syncTracker.discardDirty(.settings, ids: categories) + syncTracker.discardDirty(.settings, ids: categoriesByConnection.values.flatMap { $0 }) } } @@ -232,14 +226,8 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin var entries = loadEntries(for: key.connectionId) guard entries.removeValue(forKey: key.storageKey) != nil else { return } - if entries.isEmpty { - cache[key.connectionId] = [:] - removeFile(for: key.connectionId) - } else { - cache[key.connectionId] = entries - writeEntries(entries, for: key.connectionId) - } - syncTracker.markDeleted(.settings, id: Self.syncCategory(for: key.storageKey)) + guard store(entries, for: key.connectionId) else { return } + syncTracker.markDeleted(.settings, ids: [Self.syncCategory(for: key.storageKey)], owner: key.connectionId) } func clearGeometry(for key: ColumnLayoutTableKey) { @@ -256,14 +244,8 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin syncTracker.markDirty(.settings, id: Self.syncCategory(for: key.storageKey)) } else { entries.removeValue(forKey: key.storageKey) - if entries.isEmpty { - cache[key.connectionId] = [:] - removeFile(for: key.connectionId) - } else { - cache[key.connectionId] = entries - writeEntries(entries, for: key.connectionId) - } - syncTracker.markDeleted(.settings, id: Self.syncCategory(for: key.storageKey)) + guard store(entries, for: key.connectionId) else { return } + syncTracker.markDeleted(.settings, ids: [Self.syncCategory(for: key.storageKey)], owner: key.connectionId) } } @@ -271,6 +253,39 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin syncCategoryPrefix + storageKey } + nonisolated static func connectionId(ofSyncCategory category: String) -> UUID? { + guard category.hasPrefix(syncCategoryPrefix) else { return nil } + return TableScope(storageComponent: String(category.dropFirst(syncCategoryPrefix.count)))?.connectionId + } + + func storageKeys(forSyncRecordNames recordNames: Set) -> [String] { + let layoutPrefix = SyncRecordType.settings.recordNamePrefix + Self.syncCategoryPrefix + let digestPrefix = SyncRecordType.settings.recordNamePrefix + SyncRecordName.digestPrefix + let named = recordNames.filter { $0.hasPrefix(layoutPrefix) }.map { String($0.dropFirst(layoutPrefix.count)) } + let digested = recordNames.filter { $0.hasPrefix(digestPrefix) } + guard !digested.isEmpty else { return named } + return named + customizedStorageKeys().filter { storageKey in + digested.contains(SyncRecordType.settings.recordName(for: Self.syncCategory(for: storageKey))) + } + } + + func removeWithoutSync(storageKeys: [String]) -> Bool { + let scoped = storageKeys.compactMap { key in TableScope(storageComponent: key).map { (key, $0.connectionId) } } + var persisted = true + for (connectionId, keys) in Dictionary(grouping: scoped, by: \.1) { + var entries = loadEntries(for: connectionId) + let removed = keys.map(\.0).filter { entries.removeValue(forKey: $0) != nil } + guard !removed.isEmpty else { continue } + guard store(entries, for: connectionId) else { + persisted = false + continue + } + removed.forEach(removeLegacyHidden(storageKey:)) + syncTracker.discardDirty(.settings, ids: removed.map(Self.syncCategory(for:))) + } + return persisted + } + func rawData(forStorageKey storageKey: String) -> Data? { guard let scope = TableScope(storageComponent: storageKey), let entry = loadEntries(for: scope.connectionId)[storageKey] else { return nil } @@ -309,7 +324,18 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin } private func removeLegacyHidden(for key: ColumnLayoutTableKey) { - defaults.removeObject(forKey: Self.legacyVisibilityPrefix + key.storageKey) + removeLegacyHidden(storageKey: key.storageKey) + } + + private func removeLegacyHidden(storageKey: String) { + defaults.removeObject(forKey: Self.legacyVisibilityPrefix + storageKey) + } + + @discardableResult + private func store(_ entries: [String: PersistedColumnLayout], for connectionId: UUID) -> Bool { + let stored = entries.isEmpty ? removeFile(for: connectionId) : writeEntries(entries, for: connectionId) + cache[connectionId] = stored ? entries : nil + return stored } private func loadEntries(for connectionId: UUID) -> [String: PersistedColumnLayout] { @@ -335,27 +361,33 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin } } - private func writeEntries(_ entries: [String: PersistedColumnLayout], for connectionId: UUID) { + @discardableResult + private func writeEntries(_ entries: [String: PersistedColumnLayout], for connectionId: UUID) -> Bool { let fileURL = fileURL(for: connectionId) do { let data = try encoder.encode(entries) try data.write(to: fileURL, options: .atomic) + return true } catch { Self.logger.error( "Failed to write column layouts for \(connectionId): \(error.localizedDescription)" ) + return false } } - private func removeFile(for connectionId: UUID) { + @discardableResult + private func removeFile(for connectionId: UUID) -> Bool { let fileURL = fileURL(for: connectionId) - guard FileManager.default.fileExists(atPath: fileURL.path) else { return } + guard FileManager.default.fileExists(atPath: fileURL.path) else { return true } do { try FileManager.default.removeItem(at: fileURL) + return true } catch { Self.logger.error( "Failed to remove column layout file for \(connectionId): \(error.localizedDescription)" ) + return false } } diff --git a/TablePro/Core/Storage/ConnectionLocalState.swift b/TablePro/Core/Storage/ConnectionLocalState.swift index f6486158b2..d38ff0c93b 100644 --- a/TablePro/Core/Storage/ConnectionLocalState.swift +++ b/TablePro/Core/Storage/ConnectionLocalState.swift @@ -16,10 +16,16 @@ internal enum ConnectionLocalState { nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "ConnectionLocalState") /// Who deleted the connection. A local delete leaves tombstones so the other devices follow; - /// a remote delete must not, or it pushes back a deletion the sender already made. + /// a remote delete must not, or it pushes back a deletion the sender already made, and neither + /// may the local delete of a connection kept off iCloud. internal enum Origin { case local + case localOnly case remote + + var leavesTombstones: Bool { + self == .local + } } internal static func purge( @@ -31,7 +37,8 @@ internal enum ConnectionLocalState { favoriteDatabases: FavoriteDatabasesStorage = .shared, sqlFavorites: SQLFavoriteManager = .shared, queryHistory: QueryHistoryManager = .shared, - defaults: UserDefaults = AppStorageEnvironment.shared.defaults + defaults: UserDefaults = AppStorageEnvironment.shared.defaults, + syncTracker: SyncChangeTracker = .shared ) { guard !connectionIds.isEmpty else { return } @@ -39,7 +46,12 @@ internal enum ConnectionLocalState { purgeLiveState(connectionId) appSettings.saveLastDatabase(nil, for: connectionId) appSettings.saveLastSchema(nil, for: connectionId) - purgeFavorites(connectionId, origin: origin, tables: favoriteTables, databases: favoriteDatabases) + purgeFavorites( + connectionId, + leavesTombstones: origin.leavesTombstones, + tables: favoriteTables, + databases: favoriteDatabases + ) SidebarPersistenceKey.removeAll(connectionId: connectionId) RecentTablesStore.shared.removeEntries(for: connectionId) HistoryPanelPreferencesStorage.remove(for: connectionId) @@ -49,7 +61,11 @@ internal enum ConnectionLocalState { purgeTrailingPaneKeys(connectionIds, defaults: defaults) for store in tableScopedStores { - store.purgeConnections(connectionIds, leavesTombstones: origin == .local) + store.purgeConnections(connectionIds, leavesTombstones: origin.leavesTombstones) + } + if origin == .localOnly { + syncTracker.discardTombstones(ownedBy: connectionIds) + syncTracker.keepOffSync(owners: connectionIds) } DatabaseTreeFilterStorage.shared.removeFilters(for: connectionIds) LoadableExtensionApprovalStore.shared.revoke(for: connectionIds) @@ -57,7 +73,11 @@ internal enum ConnectionLocalState { WorkspaceRailOrderStore.shared.removeEntries(for: connectionIds) Task { await purgeAsyncStores( - connectionIds, origin: origin, sqlFavorites: sqlFavorites, queryHistory: queryHistory + connectionIds, + origin: origin, + sqlFavorites: sqlFavorites, + queryHistory: queryHistory, + syncTracker: syncTracker ) } } @@ -82,12 +102,17 @@ internal enum ConnectionLocalState { _ connectionIds: Set, origin: Origin, sqlFavorites: SQLFavoriteManager = .shared, - queryHistory: QueryHistoryManager = .shared + queryHistory: QueryHistoryManager = .shared, + syncTracker: SyncChangeTracker = .shared ) async { for connectionId in connectionIds { switch origin { case .local: await sqlFavorites.removeFavoritesAndFolders(for: connectionId) + case .localOnly: + if await sqlFavorites.removeFavoritesAndFoldersWithoutSync(for: connectionId) { + syncTracker.releaseOwnersKeptOffSync([connectionId]) + } case .remote: await sqlFavorites.removeFavoritesAndFoldersWithoutSync(for: connectionId) } @@ -129,17 +154,16 @@ internal enum ConnectionLocalState { private static func purgeFavorites( _ connectionId: UUID, - origin: Origin, + leavesTombstones: Bool, tables: FavoriteTablesStorage, databases: FavoriteDatabasesStorage ) { - switch origin { - case .local: - tables.removeFavorites(for: connectionId) - databases.removeFavorites(for: connectionId) - case .remote: + guard leavesTombstones else { tables.removeFavoritesWithoutSync(for: connectionId) databases.removeFavoritesWithoutSync(for: connectionId) + return } + tables.removeFavorites(for: connectionId) + databases.removeFavorites(for: connectionId) } } diff --git a/TablePro/Core/Storage/ConnectionStorage.swift b/TablePro/Core/Storage/ConnectionStorage.swift index 2435bb3717..8a0fafd245 100644 --- a/TablePro/Core/Storage/ConnectionStorage.swift +++ b/TablePro/Core/Storage/ConnectionStorage.swift @@ -147,7 +147,7 @@ final class ConnectionStorage { Self.logger.error("Aborted addConnection: persistence failed for \(connection.id, privacy: .public)") return } - if !connection.localOnly && !connection.isSample { + if connection.participatesInSync { syncTracker.markDirty(.connection, id: connection.id.uuidString) } @@ -165,7 +165,7 @@ final class ConnectionStorage { Self.logger.error("Aborted updateConnection: persistence failed for \(connection.id, privacy: .public)") return } - if !connection.localOnly && !connection.isSample { + if connection.participatesInSync { syncTracker.markDirty(.connection, id: connection.id.uuidString) } @@ -196,7 +196,7 @@ final class ConnectionStorage { return false } let dirtyIds = updatesById.values - .filter { !$0.localOnly && !$0.isSample } + .filter(\.participatesInSync) .map { $0.id.uuidString } syncTracker.markDirty(.connection, ids: dirtyIds) return true @@ -239,7 +239,7 @@ final class ConnectionStorage { return false } let dirtyIds = changed - .filter { !$0.localOnly && !$0.isSample } + .filter(\.participatesInSync) .map { $0.id.uuidString } syncTracker.markDirty(.connection, ids: dirtyIds) appEventsProvider().connectionUpdated.send(changed.count == 1 ? changed.first?.id : nil) @@ -321,7 +321,7 @@ final class ConnectionStorage { } let updatedConnection = connections[index] - if !updatedConnection.localOnly && !updatedConnection.isSample { + if updatedConnection.participatesInSync { syncTracker.markDirty(.connection, id: updatedConnection.id.uuidString) } @@ -337,7 +337,7 @@ final class ConnectionStorage { Self.logger.error("Aborted deleteConnection: persistence failed for \(connection.id, privacy: .public)") return false } - if !connection.localOnly && !connection.isSample { + if connection.participatesInSync { syncTracker.markDeleted(.connection, id: connection.id.uuidString) } deletePassword(for: connection.id) @@ -355,8 +355,9 @@ final class ConnectionStorage { ConnectionLocalState.purge( connectionIds: [connection.id], - origin: .local, - appSettings: appSettingsProvider() + origin: connection.participatesInSync ? .local : .localOnly, + appSettings: appSettingsProvider(), + syncTracker: syncTracker ) return true } @@ -371,7 +372,7 @@ final class ConnectionStorage { Self.logger.error("Aborted deleteConnections: persistence failed for \(idsToDelete.count, privacy: .public) connection(s)") return false } - for conn in connectionsToDelete where !conn.localOnly && !conn.isSample { + for conn in connectionsToDelete where conn.participatesInSync { syncTracker.markDeleted(.connection, id: conn.id.uuidString) } for conn in connectionsToDelete { @@ -387,10 +388,18 @@ final class ConnectionStorage { let fields = Self.secureFieldIds(for: conn.type) deleteAllPluginSecureFields(for: conn.id, fieldIds: fields) } + let syncedIds = Set(connectionsToDelete.filter(\.participatesInSync).map(\.id)) ConnectionLocalState.purge( - connectionIds: idsToDelete, + connectionIds: syncedIds, origin: .local, - appSettings: appSettingsProvider() + appSettings: appSettingsProvider(), + syncTracker: syncTracker + ) + ConnectionLocalState.purge( + connectionIds: idsToDelete.subtracting(syncedIds), + origin: .localOnly, + appSettings: appSettingsProvider(), + syncTracker: syncTracker ) return true } @@ -456,7 +465,7 @@ final class ConnectionStorage { return nil } let dirtyIds = ([placedDuplicate] + renumbered) - .filter { !$0.localOnly && !$0.isSample } + .filter(\.participatesInSync) .map { $0.id.uuidString } syncTracker.markDirty(.connection, ids: dirtyIds) diff --git a/TablePro/Core/Storage/FavoriteDatabasesStorage.swift b/TablePro/Core/Storage/FavoriteDatabasesStorage.swift index f7e9345d7b..422335c8a3 100644 --- a/TablePro/Core/Storage/FavoriteDatabasesStorage.swift +++ b/TablePro/Core/Storage/FavoriteDatabasesStorage.swift @@ -86,9 +86,10 @@ internal final class FavoriteDatabasesStorage { } } - internal func removeFavoriteWithoutSync(id: String) { + internal func removeFavoritesWithoutSync(ids: Set) { + guard !ids.isEmpty else { return } commit(sync: .discard) { favorites in - favorites = favorites.filter { Self.syncId(for: $0) != id } + favorites = favorites.filter { !ids.contains(Self.syncId(for: $0)) } } } @@ -116,6 +117,10 @@ internal final class FavoriteDatabasesStorage { case discard } + private static func syncIdsByConnection(of entries: Set) -> [UUID: Set] { + Dictionary(grouping: entries, by: \.connectionId).mapValues { Set($0.map(syncId(for:))) } + } + private static func upsert(_ entry: FavoriteDatabaseEntry, into favorites: inout Set) { guard !entry.database.isEmpty else { return } if let existing = favorites.first(where: { $0.id == entry.id }) { @@ -131,17 +136,17 @@ internal final class FavoriteDatabasesStorage { edit(&favorites) let previousById = Dictionary(previous.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) let currentIds = Set(favorites.map(\.id)) - let removedIds = previous.filter { !currentIds.contains($0.id) }.map(Self.syncId(for:)) + let removed = previous.filter { !currentIds.contains($0.id) } let changedIds = favorites.filter { previousById[$0.id] != $0 }.map(Self.syncId(for:)) - guard !removedIds.isEmpty || !changedIds.isEmpty else { return } + guard !removed.isEmpty || !changedIds.isEmpty else { return } persist(favorites) switch sync { case .track: - syncTracker.markDeleted(.favoriteDatabase, ids: removedIds) + syncTracker.markDeleted(.favoriteDatabase, idsByOwner: Self.syncIdsByConnection(of: removed)) syncTracker.markDirty(.favoriteDatabase, ids: changedIds) case .discard: - syncTracker.discardDirty(.favoriteDatabase, ids: removedIds) + syncTracker.discardDirty(.favoriteDatabase, ids: removed.map(Self.syncId(for:))) } postChangeNotification() } diff --git a/TablePro/Core/Storage/FavoriteTablesStorage.swift b/TablePro/Core/Storage/FavoriteTablesStorage.swift index fe5f9ddaf4..b7ea4100f1 100644 --- a/TablePro/Core/Storage/FavoriteTablesStorage.swift +++ b/TablePro/Core/Storage/FavoriteTablesStorage.swift @@ -156,7 +156,7 @@ final class FavoriteTablesStorage: @unchecked Sendable { @MainActor @discardableResult - func applyRemote(saved: [FavoriteEntry], deletedIds: Set) -> Set { + func applyRemote(saved: [FavoriteEntry], deletedIds: Set) -> [UUID: Set] { var removedThroughAliases: Set = [] let change = mutateState { favorites in favorites.formUnion(saved) @@ -166,7 +166,7 @@ final class FavoriteTablesStorage: @unchecked Sendable { if change.changesEntries { NotificationCenter.default.post(name: .favoriteTablesDidChange, object: self) } - return Set(removedThroughAliases.map(Self.syncId(for:))) + return Self.syncIdsByConnection(of: removedThroughAliases) } @MainActor @@ -222,14 +222,14 @@ final class FavoriteTablesStorage: @unchecked Sendable { remaining = favorites } guard change.changesEntries else { return change } - let removedIds = Self.recordIds(of: change.removed, keepingAliasesOf: remaining) + let removedIds = Self.recordIdsByConnection(of: change.removed, keepingAliasesOf: remaining) switch sync { case .track: let reclaimedAliases = Self.legacyAliases(of: change.removed).intersection(Self.legacyAliases(of: remaining)) - syncTracker.markDeleted(.tableFavorite, ids: Array(removedIds)) + syncTracker.markDeleted(.tableFavorite, idsByOwner: removedIds) syncTracker.markDirty(.tableFavorite, ids: Array(Self.recordIds(of: change.added).union(reclaimedAliases))) case .discard: - syncTracker.discardDirty(.tableFavorite, ids: Array(removedIds)) + syncTracker.discardDirty(.tableFavorite, ids: removedIds.values.flatMap { $0 }) } NotificationCenter.default.post(name: .favoriteTablesDidChange, object: self) return change @@ -253,13 +253,22 @@ final class FavoriteTablesStorage: @unchecked Sendable { Set(favorites.map(syncId(for:))).union(legacyAliases(of: favorites)) } - private static func recordIds( + private static func recordIdsByConnection( of removed: Set, keepingAliasesOf remaining: Set - ) -> Set { + ) -> [UUID: Set] { let claimedAliases = legacyAliases(of: remaining) - let retiredAliases = legacyAliases(of: removed).subtracting(claimedAliases) - return Set(removed.map(syncId(for:))).union(retiredAliases) + var ids: [UUID: Set] = [:] + for entry in removed { + ids[entry.connectionId, default: []].insert(syncId(for: entry)) + guard let alias = legacyAlias(of: entry), !claimedAliases.contains(alias) else { continue } + ids[entry.connectionId, default: []].insert(alias) + } + return ids + } + + private static func syncIdsByConnection(of entries: Set) -> [UUID: Set] { + Dictionary(grouping: entries, by: \.connectionId).mapValues { Set($0.map(syncId(for:))) } } private static func remove(_ deletedIds: Set, from favorites: inout Set) -> Set { diff --git a/TablePro/Core/Storage/SQLFavoriteManager.swift b/TablePro/Core/Storage/SQLFavoriteManager.swift index 85d91d4e04..2559d2123c 100644 --- a/TablePro/Core/Storage/SQLFavoriteManager.swift +++ b/TablePro/Core/Storage/SQLFavoriteManager.swift @@ -45,8 +45,8 @@ internal final class SQLFavoriteManager: @unchecked Sendable { func deleteFavorite(id: UUID) async -> Bool { await operations.run { [self] in - guard await storage.deleteFavorite(id: id) else { return false } - syncTracker.markDeleted(.favorite, id: id.uuidString) + guard let connectionIds = await storage.deleteFavorites(ids: [id]) else { return false } + markDeleted(.favorite, ids: [id], connectionIds: connectionIds) postUpdateNotification(connectionId: nil) return true } @@ -54,10 +54,8 @@ internal final class SQLFavoriteManager: @unchecked Sendable { func deleteFavorites(ids: [UUID]) async { await operations.run { [self] in - guard await storage.deleteFavorites(ids: ids) else { return } - for id in ids { - syncTracker.markDeleted(.favorite, id: id.uuidString) - } + guard let connectionIds = await storage.deleteFavorites(ids: ids) else { return } + markDeleted(.favorite, ids: ids, connectionIds: connectionIds) postUpdateNotification(connectionId: nil) } } @@ -72,14 +70,10 @@ internal final class SQLFavoriteManager: @unchecked Sendable { /// holding the record re-uploads what was just deleted. func removeFavoritesAndFolders(for connectionId: UUID) async { await operations.run { [self] in - let removed = await storage.deleteFavoritesAndFolders(connectionId: connectionId) - guard !removed.isEmpty else { return } - for id in removed.favorites { - syncTracker.markDeleted(.favorite, id: id.uuidString) - } - for id in removed.folders { - syncTracker.markDeleted(.favoriteFolder, id: id.uuidString) - } + guard let removed = await storage.deleteFavoritesAndFolders(connectionId: connectionId), + !removed.isEmpty else { return } + syncTracker.markDeleted(.favorite, ids: removed.favorites.map(\.uuidString), owner: connectionId) + syncTracker.markDeleted(.favoriteFolder, ids: removed.folders.map(\.uuidString), owner: connectionId) markDetachedDirty(removed.detached) postUpdateNotification(connectionId: nil) } @@ -99,16 +93,26 @@ internal final class SQLFavoriteManager: @unchecked Sendable { syncTracker.markDirty(.favoriteFolder, ids: detached.folders.map(\.uuidString)) } + @MainActor + private func markDeleted(_ type: SyncRecordType, ids: [UUID], connectionIds: [UUID: UUID]) { + let idsByConnection = Dictionary(grouping: ids) { connectionIds[$0] } + for (connectionId, deletedIds) in idsByConnection { + syncTracker.markDeleted(type, ids: deletedIds.map(\.uuidString), owner: connectionId) + } + } + /// Used when another device deleted the connection. Marking tombstones here would push its own /// deletion straight back at it, which is the reason `FavoriteTablesStorage` splits the same /// way. - func removeFavoritesAndFoldersWithoutSync(for connectionId: UUID) async { + @discardableResult + func removeFavoritesAndFoldersWithoutSync(for connectionId: UUID) async -> Bool { await operations.run { [self] in - let removed = await storage.deleteFavoritesAndFolders(connectionId: connectionId) - guard !removed.isEmpty else { return } + guard let removed = await storage.deleteFavoritesAndFolders(connectionId: connectionId) else { return false } + guard !removed.isEmpty else { return true } syncTracker.discardDirty(.favorite, ids: removed.favorites.map(\.uuidString)) syncTracker.discardDirty(.favoriteFolder, ids: removed.folders.map(\.uuidString)) postUpdateNotification(connectionId: nil) + return true } } @@ -195,7 +199,7 @@ internal final class SQLFavoriteManager: @unchecked Sendable { func deleteFolder(id: UUID) async -> Bool { await operations.run { [self] in guard let deletion = await storage.deleteFolder(id: id) else { return false } - syncTracker.markDeleted(.favoriteFolder, id: id.uuidString) + syncTracker.markDeleted(.favoriteFolder, ids: [id.uuidString], owner: deletion.connectionId) syncTracker.markDirty(.favorite, ids: deletion.movedFavorites.map(\.uuidString)) syncTracker.markDirty(.favoriteFolder, ids: deletion.movedFolders.map(\.uuidString)) postUpdateNotification(connectionId: nil) @@ -286,7 +290,7 @@ internal final class SQLFavoriteManager: @unchecked Sendable { @MainActor private func applyRemoteFavoriteDeletions(_ ids: Set) async -> Bool { guard !ids.isEmpty else { return true } - guard await storage.deleteFavorites(ids: Array(ids)) else { return false } + guard await storage.deleteFavorites(ids: Array(ids)) != nil else { return false } syncTracker.discardDirty(.favorite, ids: ids.map(\.uuidString)) postUpdateNotification(connectionId: nil) return true diff --git a/TablePro/Core/Storage/SQLFavoriteStorage+Deletion.swift b/TablePro/Core/Storage/SQLFavoriteStorage+Deletion.swift new file mode 100644 index 0000000000..26c0cfcb61 --- /dev/null +++ b/TablePro/Core/Storage/SQLFavoriteStorage+Deletion.swift @@ -0,0 +1,44 @@ +import Foundation +import SQLite3 + +internal extension SQLFavoriteStorage { + func deleteFavorites(ids: [UUID]) -> [UUID: UUID]? { + guard !ids.isEmpty else { return [:] } + let placeholders = ids.map { _ in "?" }.joined(separator: ",") + let bindings = ids.map(\.uuidString) + + guard let connectionIds = connectionIds( + of: "SELECT id, connection_id FROM favorites WHERE id IN (\(placeholders)) AND connection_id IS NOT NULL;", + bindings: bindings + ), run("DELETE FROM favorites WHERE id IN (\(placeholders));", bindings: bindings) else { + return nil + } + return connectionIds + } + + private func connectionIds(of sql: String, bindings: [String]) -> [UUID: UUID]? { + var statement: OpaquePointer? + guard sqlite3_prepare_v2(db, sql, -1, &statement, nil) == SQLITE_OK else { return nil } + defer { sqlite3_finalize(statement) } + let transient = unsafeBitCast(-1, to: sqlite3_destructor_type.self) + for (index, value) in bindings.enumerated() { + sqlite3_bind_text(statement, Int32(index + 1), value, -1, transient) + } + + var connectionIds: [UUID: UUID] = [:] + while true { + switch sqlite3_step(statement) { + case SQLITE_ROW: + guard let rawId = sqlite3_column_text(statement, 0), + let rawConnectionId = sqlite3_column_text(statement, 1), + let id = UUID(uuidString: String(cString: rawId)), + let connectionId = UUID(uuidString: String(cString: rawConnectionId)) else { continue } + connectionIds[id] = connectionId + case SQLITE_DONE: + return connectionIds + default: + return nil + } + } + } +} diff --git a/TablePro/Core/Storage/SQLFavoriteStorage.swift b/TablePro/Core/Storage/SQLFavoriteStorage.swift index 227e61238d..cb914a2682 100644 --- a/TablePro/Core/Storage/SQLFavoriteStorage.swift +++ b/TablePro/Core/Storage/SQLFavoriteStorage.swift @@ -460,45 +460,6 @@ internal actor SQLFavoriteStorage { return result } - func deleteFavorite(id: UUID) -> Bool { - let sql = "DELETE FROM favorites WHERE id = ?;" - var statement: OpaquePointer? - guard sqlite3_prepare_v2(db, sql, -1, &statement, nil) == SQLITE_OK else { - return false - } - - defer { sqlite3_finalize(statement) } - - let SQLITE_TRANSIENT = unsafeBitCast(-1, to: sqlite3_destructor_type.self) - sqlite3_bind_text(statement, 1, id.uuidString, -1, SQLITE_TRANSIENT) - return sqlite3_step(statement) == SQLITE_DONE - } - - func deleteFavorites(ids: [UUID]) -> Bool { - guard !ids.isEmpty else { return true } - - let placeholders = ids.map { _ in "?" }.joined(separator: ",") - let sql = "DELETE FROM favorites WHERE id IN (\(placeholders));" - - var statement: OpaquePointer? - guard sqlite3_prepare_v2(db, sql, -1, &statement, nil) == SQLITE_OK else { - return false - } - - defer { sqlite3_finalize(statement) } - - let SQLITE_TRANSIENT = unsafeBitCast(-1, to: sqlite3_destructor_type.self) - for (index, id) in ids.enumerated() { - sqlite3_bind_text(statement, Int32(index + 1), id.uuidString, -1, SQLITE_TRANSIENT) - } - - let result = sqlite3_step(statement) - if result != SQLITE_DONE { - Self.logger.error("Failed to batch delete favorites: \(String(cString: sqlite3_errmsg(self.db)))") - } - return result == SQLITE_DONE - } - /// Both tables point at `folders` by id with no foreign key behind either column, so a delete /// that removes a folder leaves whatever named it holding an id nothing answers to. /// @@ -542,8 +503,8 @@ internal actor SQLFavoriteStorage { /// each record for sync and cannot ask afterwards: the rows are gone. Reporting a bare `Bool` /// is why a deleted connection's favorites and folders lived on in CloudKit and came back on a /// fresh install. - func deleteFavoritesAndFolders(connectionId: UUID) -> DeletedFavoriteRecords { - guard sqlite3_exec(db, "BEGIN IMMEDIATE;", nil, nil, nil) == SQLITE_OK else { return .none } + func deleteFavoritesAndFolders(connectionId: UUID) -> DeletedFavoriteRecords? { + guard sqlite3_exec(db, "BEGIN IMMEDIATE;", nil, nil, nil) == SQLITE_OK else { return nil } let id = connectionId.uuidString /// Read inside the same transaction as the delete, so nothing can be added between the two @@ -555,10 +516,10 @@ internal actor SQLFavoriteStorage { run("DELETE FROM folders WHERE connection_id = ?;", bindings: [id]), let detached = detachDanglingFolderReferences() else { sqlite3_exec(db, "ROLLBACK;", nil, nil, nil) - return .none + return nil } - guard sqlite3_exec(db, "COMMIT;", nil, nil, nil) == SQLITE_OK else { return .none } + guard sqlite3_exec(db, "COMMIT;", nil, nil, nil) == SQLITE_OK else { return nil } return DeletedFavoriteRecords(favorites: favorites, folders: folders, detached: detached) } @@ -602,7 +563,7 @@ internal actor SQLFavoriteStorage { return result } - private func run(_ sql: String, bindings: [String] = []) -> Bool { + func run(_ sql: String, bindings: [String] = []) -> Bool { var statement: OpaquePointer? guard sqlite3_prepare_v2(db, sql, -1, &statement, nil) == SQLITE_OK else { Self.logger.error("Failed to prepare statement: \(String(cString: sqlite3_errmsg(self.db)))") @@ -929,7 +890,7 @@ internal actor SQLFavoriteStorage { let SQLITE_TRANSIENT = unsafeBitCast(-1, to: sqlite3_destructor_type.self) - let findParentSQL = "SELECT parent_id FROM folders WHERE id = ?;" + let findParentSQL = "SELECT parent_id, connection_id FROM folders WHERE id = ?;" var findStatement: OpaquePointer? guard sqlite3_prepare_v2(db, findParentSQL, -1, &findStatement, nil) == SQLITE_OK else { sqlite3_exec(db, "ROLLBACK;", nil, nil, nil) @@ -939,8 +900,10 @@ internal actor SQLFavoriteStorage { sqlite3_bind_text(findStatement, 1, idString, -1, SQLITE_TRANSIENT) var parentId: String? + var connectionId: UUID? if sqlite3_step(findStatement) == SQLITE_ROW { parentId = sqlite3_column_text(findStatement, 0).map { String(cString: $0) } + connectionId = sqlite3_column_text(findStatement, 1).flatMap { UUID(uuidString: String(cString: $0)) } } sqlite3_finalize(findStatement) @@ -1003,7 +966,7 @@ internal actor SQLFavoriteStorage { } guard sqlite3_exec(db, "COMMIT;", nil, nil, nil) == SQLITE_OK else { return nil } - return FolderDeletion(movedFavorites: movedFavorites, movedFolders: movedFolders) + return FolderDeletion(connectionId: connectionId, movedFavorites: movedFavorites, movedFolders: movedFolders) } func fetchFolders(connectionId: UUID? = nil) -> [SQLFavoriteFolder] { @@ -1264,6 +1227,7 @@ enum FavoriteScopeRead: Equatable { /// What deleting one folder moved up to its parent, so the caller can mark those records dirty. struct FolderDeletion: Equatable { + let connectionId: UUID? let movedFavorites: [UUID] let movedFolders: [UUID] } @@ -1294,8 +1258,6 @@ struct DeletedFavoriteRecords { self.detached = detached } - static let none = DeletedFavoriteRecords(favorites: [], folders: []) - var isEmpty: Bool { favorites.isEmpty && folders.isEmpty && detached.isEmpty } diff --git a/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift b/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift index f35bef2c21..762b721d0f 100644 --- a/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift +++ b/TablePro/Core/Sync/Extensions/SyncCoordinator+PushCollection.swift @@ -13,76 +13,106 @@ struct SyncPushBatch { } } +enum SyncPushDisposition { + case push(CKRecord) + case hold + case drop +} + private struct BuiltRecord { let id: String let record: CKRecord } +private struct CollectedRecords { + private(set) var built: [BuiltRecord] = [] + private(set) var heldIds: Set = [] + private var resolvedIds: Set = [] + + func hasResolved(_ id: String) -> Bool { + resolvedIds.contains(id) + } + + mutating func add(_ id: String, _ disposition: SyncPushDisposition) { + switch disposition { + case .push(let record): + built.append(BuiltRecord(id: id, record: record)) + case .hold: + heldIds.insert(id) + case .drop: + return + } + resolvedIds.insert(id) + } +} + extension SyncCoordinator { func collectPushBatch( snapshot: SyncEditSnapshot, - settings: SyncSettings, + boundary: SyncBoundary, zoneID: CKRecordZone.ID ) async -> SyncPushBatch { var batch = SyncPushBatch() - if settings.syncConnections { + if boundary.includes(.connection) { let storage = services.connectionStorage - await collectRecords(of: .connection, snapshot: snapshot, into: &batch, zoneID: zoneID) { + await collectRecords(of: .connection, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) { let connections = storage.loadConnections() return storage.lastLoadFailed ? nil : connections - } record: { (connection: DatabaseConnection) -> CKRecord? in - guard !connection.localOnly else { return nil } + } disposition: { (connection: DatabaseConnection) in + guard connection.participatesInSync else { return .drop } let recordID = SyncRecordMapper.recordID(type: .connection, id: connection.id.uuidString, in: zoneID) - return SyncRecordMapper.toCKRecord(connection, in: zoneID, base: recordCache.record(for: recordID)) + return .push(SyncRecordMapper.toCKRecord(connection, in: zoneID, base: recordCache.record(for: recordID))) } } - if settings.syncGroupsAndTags { + if boundary.includes(.group) { let groupStorage = services.groupStorage - await collectRecords(of: .group, snapshot: snapshot, into: &batch, zoneID: zoneID) { + await collectRecords(of: .group, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) { let groups = groupStorage.loadGroups() return groupStorage.storeIsUnreadable ? nil : groups - } record: { (group: ConnectionGroup) in SyncRecordMapper.toCKRecord(group, in: zoneID) } + } disposition: { (group: ConnectionGroup) in .push(SyncRecordMapper.toCKRecord(group, in: zoneID)) } + } + if boundary.includes(.tag) { let tagStorage = services.tagStorage - await collectRecords(of: .tag, snapshot: snapshot, into: &batch, zoneID: zoneID) { + await collectRecords(of: .tag, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) { let tags = tagStorage.loadTags() return tagStorage.storeIsUnreadable ? nil : tags - } record: { (tag: ConnectionTag) in SyncRecordMapper.toCKRecord(tag, in: zoneID) } + } disposition: { (tag: ConnectionTag) in .push(SyncRecordMapper.toCKRecord(tag, in: zoneID)) } } - if settings.syncSSHProfiles { + if boundary.includes(.sshProfile) { let storage = services.sshProfileStorage - await collectRecords(of: .sshProfile, snapshot: snapshot, into: &batch, zoneID: zoneID) { + await collectRecords(of: .sshProfile, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) { let profiles = storage.loadProfiles() return storage.lastLoadFailed ? nil : profiles - } record: { (profile: SSHProfile) in SyncRecordMapper.toCKRecord(profile, in: zoneID) } + } disposition: { (profile: SSHProfile) in .push(SyncRecordMapper.toCKRecord(profile, in: zoneID)) } } - if settings.syncCredentialProfiles { + if boundary.includes(.credentialProfile) { let storage = services.credentialProfileStorage - await collectRecords(of: .credentialProfile, snapshot: snapshot, into: &batch, zoneID: zoneID) { + await collectRecords( + of: .credentialProfile, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID + ) { let profiles = storage.loadProfiles() return storage.lastLoadFailed ? nil : profiles - } record: { (profile: CredentialProfile) in SyncRecordMapper.toCKRecord(profile, in: zoneID) } + } disposition: { (profile: CredentialProfile) in .push(SyncRecordMapper.toCKRecord(profile, in: zoneID)) } } - if settings.syncSettings { - collectSettings(snapshot: snapshot, into: &batch, zoneID: zoneID) + if boundary.includes(.settings) { + collectSettings(snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) } - if settings.syncTableFavorites { - collectTableFavorites(snapshot: snapshot, into: &batch, zoneID: zoneID) + if boundary.includes(.tableFavorite) { + collectTableFavorites(snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) } - if settings.syncDatabaseFavorites { - collectDatabaseFavorites(snapshot: snapshot, into: &batch, zoneID: zoneID) + if boundary.includes(.favoriteDatabase) { + collectDatabaseFavorites(snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) } - if settings.syncSQLFavorites { - await collectSQLFavorites(snapshot: snapshot, into: &batch, zoneID: zoneID) - } + await collectSQLFavorites(snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) return batch } @@ -90,55 +120,54 @@ extension SyncCoordinator { private func collectRecords( of type: SyncRecordType, snapshot: SyncEditSnapshot, + boundary: SyncBoundary, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID, loaded: () async -> [Record]?, - record: (Record) -> CKRecord? + disposition: (Record) -> SyncPushDisposition ) async where Record.ID == UUID { let dirtyIds = snapshot.dirtyIds(for: type) guard !dirtyIds.isEmpty else { - append(type, dirtyIds: dirtyIds, built: [], into: &batch, zoneID: zoneID) + append(type, dirtyIds: dirtyIds, collected: CollectedRecords(), boundary: boundary, into: &batch, zoneID: zoneID) return } - let built = await loaded().map { records in - Self.build(records, dirtyIds: dirtyIds, id: { $0.id.uuidString }, record: record) + let collected = await loaded().map { records in + Self.collect(records, dirtyIds: dirtyIds, id: { $0.id.uuidString }, disposition: disposition) } - append(type, dirtyIds: dirtyIds, built: built, into: &batch, zoneID: zoneID) + append(type, dirtyIds: dirtyIds, collected: collected, boundary: boundary, into: &batch, zoneID: zoneID) } - private static func build( + private static func collect( _ items: [Item], dirtyIds: Set, id: (Item) -> String, - record: (Item) -> CKRecord? - ) -> [BuiltRecord] { - var built: [BuiltRecord] = [] - var builtIds: Set = [] + disposition: (Item) -> SyncPushDisposition + ) -> CollectedRecords { + var collected = CollectedRecords() for item in items { let itemId = id(item) - guard dirtyIds.contains(itemId), !builtIds.contains(itemId), let record = record(item) else { continue } - builtIds.insert(itemId) - built.append(BuiltRecord(id: itemId, record: record)) + guard dirtyIds.contains(itemId), !collected.hasResolved(itemId) else { continue } + collected.add(itemId, disposition(item)) } - return built + return collected } private func append( _ type: SyncRecordType, dirtyIds: Set, - built: [BuiltRecord]?, - retaining retainedIds: Set = [], + collected: CollectedRecords?, + boundary: SyncBoundary, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) { - guard let built else { - appendTombstones(of: type, sparing: dirtyIds, to: &batch, zoneID: zoneID) + guard let collected else { + appendTombstones(of: type, sparing: dirtyIds, boundary: boundary, to: &batch, zoneID: zoneID) return } - let pushable = Set(built.map(\.id)) - batch.records.append(contentsOf: built.map(\.record)) - discardUnpushable(type, dirtyIds: dirtyIds.subtracting(retainedIds), pushable: pushable) - let superseded = appendTombstones(of: type, sparing: pushable, to: &batch, zoneID: zoneID) + let pushable = Set(collected.built.map(\.id)) + batch.records.append(contentsOf: collected.built.map(\.record)) + discardUnpushable(type, dirtyIds: dirtyIds.subtracting(collected.heldIds), pushable: pushable) + let superseded = appendTombstones(of: type, sparing: pushable, boundary: boundary, to: &batch, zoneID: zoneID) batch.supersededTombstones.formUnion(superseded.map { SyncRecordIdentity(type: type, id: $0) }) } @@ -155,115 +184,136 @@ extension SyncCoordinator { private func appendTombstones( of type: SyncRecordType, sparing sparedIds: Set, + boundary: SyncBoundary, to batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) -> Set { var spared: Set = [] + var heldCount = 0 for tombstone in metadataStorage.tombstones(for: type) { guard !sparedIds.contains(tombstone.id) else { spared.insert(tombstone.id) continue } + guard boundary.includes(tombstone, of: type) else { + heldCount += 1 + continue + } batch.deletions.append(SyncRecordMapper.recordID(type: type, id: tombstone.id, in: zoneID)) } + if heldCount > 0 { + Self.logger.info( + "Held \(heldCount, privacy: .public) \(type.rawValue, privacy: .public) deletions of connections this Mac does not sync" + ) + } return spared } - private func collectSettings(snapshot: SyncEditSnapshot, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID) { - for category in snapshot.dirtyIds(for: .settings) { - guard let data = settingsData(for: category) else { continue } - batch.records.append(SyncRecordMapper.toCKRecord(category: category, settingsData: data, in: zoneID)) + private func collectSettings( + snapshot: SyncEditSnapshot, + boundary: SyncBoundary, + into batch: inout SyncPushBatch, + zoneID: CKRecordZone.ID + ) { + let dirtyIds = snapshot.dirtyIds(for: .settings) + retireSettingsTombstonesOfLiveCategories(dirtyIds: dirtyIds) + let collected = Self.collect(Array(dirtyIds), dirtyIds: dirtyIds, id: { $0 }) { category in + let owner = SyncBoundary.owner(ofRecordId: category, type: .settings) + guard boundary.includes(.settings, owner: owner) else { return .hold } + guard let data = settingsData(for: category) else { return .drop } + return .push(SyncRecordMapper.toCKRecord(category: category, settingsData: data, in: zoneID)) + } + append(.settings, dirtyIds: dirtyIds, collected: collected, boundary: boundary, into: &batch, zoneID: zoneID) + } + + private func retireSettingsTombstonesOfLiveCategories(dirtyIds: Set) { + let live = metadataStorage.tombstones(for: .settings) + .map(\.id) + .filter { !dirtyIds.contains($0) && settingsData(for: $0) != nil } + for category in Set(live) { + metadataStorage.removeTombstone(category, type: .settings) } } private func collectTableFavorites( snapshot: SyncEditSnapshot, + boundary: SyncBoundary, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) { let dirtyIds = snapshot.dirtyIds(for: .tableFavorite) - guard !dirtyIds.isEmpty else { - append(.tableFavorite, dirtyIds: dirtyIds, built: [], into: &batch, zoneID: zoneID) - return - } - let built = Self.buildTableFavorites( - services.favoriteTablesStorage.loadFavorites(), - dirtyIds: dirtyIds, - zoneID: zoneID - ) - append(.tableFavorite, dirtyIds: dirtyIds, built: built, into: &batch, zoneID: zoneID) + let favorites = dirtyIds.isEmpty ? [] : services.favoriteTablesStorage.loadFavorites() + let collected = Self.collectTableFavorites(favorites, dirtyIds: dirtyIds, boundary: boundary, zoneID: zoneID) + append(.tableFavorite, dirtyIds: dirtyIds, collected: collected, boundary: boundary, into: &batch, zoneID: zoneID) } - private static func buildTableFavorites( + private static func collectTableFavorites( _ favorites: Set, dirtyIds: Set, + boundary: SyncBoundary, zoneID: CKRecordZone.ID - ) -> [BuiltRecord] { + ) -> CollectedRecords { let claims = FavoriteTablesStorage.aliasClaims(in: favorites) - var built: [BuiltRecord] = [] + var collected = CollectedRecords() for entry in favorites { - let currentId = FavoriteTablesStorage.syncId(for: entry) - if dirtyIds.contains(currentId) { - built.append(BuiltRecord( - id: currentId, - record: SyncRecordMapper.toCKRecord(favoriteEntry: entry, recordId: currentId, in: zoneID) - )) + var recordIds = [FavoriteTablesStorage.syncId(for: entry)] + if let alias = FavoriteTablesStorage.legacyAlias(of: entry), claims[alias]?.count == 1 { + recordIds.append(alias) + } + let admitted = boundary.includes(.tableFavorite, owner: entry.connectionId) + for recordId in recordIds where dirtyIds.contains(recordId) { + collected.add(recordId, admitted ? .push( + SyncRecordMapper.toCKRecord(favoriteEntry: entry, recordId: recordId, in: zoneID) + ) : .hold) } - guard let alias = FavoriteTablesStorage.legacyAlias(of: entry), - dirtyIds.contains(alias), - claims[alias]?.count == 1 else { continue } - built.append(BuiltRecord( - id: alias, - record: SyncRecordMapper.toCKRecord(favoriteEntry: entry, recordId: alias, in: zoneID) - )) } - return built + return collected } - /// A connection the user marked local only never reaches iCloud, and neither do the database - /// names hanging off it. Tombstones are not filtered: a deletion only ever removes something, - /// and a connection can be marked local only after its favorites were already pushed. private func collectDatabaseFavorites( snapshot: SyncEditSnapshot, + boundary: SyncBoundary, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) { let dirtyIds = snapshot.dirtyIds(for: .favoriteDatabase) - guard !dirtyIds.isEmpty else { - append(.favoriteDatabase, dirtyIds: dirtyIds, built: [], into: &batch, zoneID: zoneID) - return - } - let localOnlyIds = Set(services.connectionStorage.loadConnections().filter(\.localOnly).map(\.id)) - let favorites = services.favoriteDatabasesStorage.loadFavorites() - let withheld = favorites.filter { localOnlyIds.contains($0.connectionId) } - let built = Self.build( - Array(favorites.subtracting(withheld)), - dirtyIds: dirtyIds, - id: FavoriteDatabasesStorage.syncId(for:), - record: { SyncRecordMapper.toCKRecord(favoriteDatabase: $0, in: zoneID) } - ) - append( - .favoriteDatabase, + let favorites = dirtyIds.isEmpty ? [] : services.favoriteDatabasesStorage.loadFavorites() + let collected = Self.collect( + Array(favorites), dirtyIds: dirtyIds, - built: built, - retaining: Set(withheld.map(FavoriteDatabasesStorage.syncId(for:))), - into: &batch, - zoneID: zoneID - ) + id: FavoriteDatabasesStorage.syncId(for:) + ) { entry in + guard boundary.includes(.favoriteDatabase, owner: entry.connectionId) else { return .hold } + return .push(SyncRecordMapper.toCKRecord(favoriteDatabase: entry, in: zoneID)) + } + append(.favoriteDatabase, dirtyIds: dirtyIds, collected: collected, boundary: boundary, into: &batch, zoneID: zoneID) } private func collectSQLFavorites( snapshot: SyncEditSnapshot, + boundary: SyncBoundary, into batch: inout SyncPushBatch, zoneID: CKRecordZone.ID ) async { let manager = services.sqlFavoriteManager - await collectRecords(of: .favorite, snapshot: snapshot, into: &batch, zoneID: zoneID) { - await manager.favoritesForSync() - } record: { (favorite: SQLFavorite) in SyncRecordMapper.toCKRecord(sqlFavorite: favorite, in: zoneID) } + if boundary.includes(.favorite) { + await collectRecords(of: .favorite, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID) { + await manager.favoritesForSync() + } disposition: { (favorite: SQLFavorite) in + guard boundary.includes(.favorite, owner: favorite.connectionId) else { return .hold } + return .push(SyncRecordMapper.toCKRecord(sqlFavorite: favorite, in: zoneID)) + } + } - await collectRecords(of: .favoriteFolder, snapshot: snapshot, into: &batch, zoneID: zoneID) { - await manager.foldersForSync() - } record: { (folder: SQLFavoriteFolder) in SyncRecordMapper.toCKRecord(sqlFavoriteFolder: folder, in: zoneID) } + if boundary.includes(.favoriteFolder) { + await collectRecords( + of: .favoriteFolder, snapshot: snapshot, boundary: boundary, into: &batch, zoneID: zoneID + ) { + await manager.foldersForSync() + } disposition: { (folder: SQLFavoriteFolder) in + guard boundary.includes(.favoriteFolder, owner: folder.connectionId) else { return .hold } + return .push(SyncRecordMapper.toCKRecord(sqlFavoriteFolder: folder, in: zoneID)) + } + } } } diff --git a/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift b/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift index a3b12d043d..b402d81e0a 100644 --- a/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift +++ b/TablePro/Core/Sync/Extensions/SyncCoordinator+RemoteDeletions.swift @@ -10,6 +10,8 @@ struct SyncPendingDeletions: Equatable { var sshProfiles: Set = [] var credentialProfiles: Set = [] var tableFavorites: Set = [] + var databaseFavorites: Set = [] + var settingsRecordNames: Set = [] var sqlFavorites: Set = [] var sqlFolders: Set = [] @@ -38,8 +40,10 @@ struct SyncPendingDeletions: Equatable { if let uuid { credentialProfiles.insert(uuid) } case .tableFavorite: tableFavorites.insert(id) - case .favoriteDatabase, .settings: - return + case .favoriteDatabase: + databaseFavorites.insert(id) + case .settings: + settingsRecordNames.insert(type.recordNamePrefix + id) case .favorite: if let uuid { sqlFavorites.insert(uuid) } case .favoriteFolder: @@ -52,7 +56,7 @@ struct SyncRemoteDeletionEffects { var connectionsChanged = false var groupsOrTagsChanged = false var persistenceFailed = false - var tableFavoriteIdsToRetire: Set = [] + var tableFavoriteIdsToRetire: [UUID: Set] = [:] } extension SyncCoordinator { @@ -67,13 +71,15 @@ extension SyncCoordinator { saved: tableFavorites, deletedIds: pending.tableFavorites ) + services.favoriteDatabasesStorage.removeFavoritesWithoutSync(ids: pending.databaseFavorites) let persisted = [ applyRemoteConnectionDeletions(pending.connections), applyRemoteGroupDeletions(pending.groups), applyRemoteTagDeletions(pending.tags), applyRemoteSSHProfileDeletions(pending.sshProfiles), - applyRemoteCredentialProfileDeletions(pending.credentialProfiles) + applyRemoteCredentialProfileDeletions(pending.credentialProfiles), + applyRemoteColumnLayoutDeletions(pending.settingsRecordNames) ] effects.persistenceFailed = persisted.contains(false) return effects @@ -101,6 +107,12 @@ extension SyncCoordinator { return true } + private func applyRemoteColumnLayoutDeletions(_ recordNames: Set) -> Bool { + guard !recordNames.isEmpty else { return true } + let persister = columnLayouts() + return persister.removeWithoutSync(storageKeys: persister.storageKeys(forSyncRecordNames: recordNames)) + } + private func applyRemoteGroupDeletions(_ ids: Set) -> Bool { guard !ids.isEmpty else { return true } var groups = services.groupStorage.loadGroups() diff --git a/TablePro/Core/Sync/Extensions/SyncCoordinator+Scope.swift b/TablePro/Core/Sync/Extensions/SyncCoordinator+Scope.swift new file mode 100644 index 0000000000..0a97545646 --- /dev/null +++ b/TablePro/Core/Sync/Extensions/SyncCoordinator+Scope.swift @@ -0,0 +1,22 @@ +import Foundation +import TableProSyncTransport + +extension SyncCoordinator { + func syncBoundary(settings: SyncSettings) -> SyncBoundary { + let storage = services.connectionStorage + let connections = storage.loadConnections() + return SyncBoundary( + settings: settings, + connections: storage.lastLoadFailed ? nil : connections, + ownersKeptOffSync: changeTracker.ownersKeptOffSync + ) + } + + func pruneTombstones(within boundary: SyncBoundary) { + metadataStorage.pruneTombstones(olderThan: Self.tombstoneRetentionDays) { type, tombstone in + boundary.includes(tombstone, of: type) + } + } + + private static let tombstoneRetentionDays = 30 +} diff --git a/TablePro/Core/Sync/SyncBoundary.swift b/TablePro/Core/Sync/SyncBoundary.swift new file mode 100644 index 0000000000..7047f282d9 --- /dev/null +++ b/TablePro/Core/Sync/SyncBoundary.swift @@ -0,0 +1,60 @@ +import Foundation +import TableProSyncTransport + +struct SyncBoundary: Equatable, Sendable { + let includedTypes: Set + let excludedConnectionIds: Set? + + init(includedTypes: Set, excludedConnectionIds: Set?) { + self.includedTypes = includedTypes + self.excludedConnectionIds = excludedConnectionIds + } + + init( + settings: SyncSettings, + connections: [DatabaseConnection]?, + ownersKeptOffSync: Set = [], + writableTypes: Set = SyncRecordType.verifiedInProduction + ) { + self.init( + includedTypes: Set(SyncRecordType.allCases.filter { settings.syncs($0) && writableTypes.contains($0) }), + excludedConnectionIds: connections.map { connections in + let keptOff = connections.filter { !$0.participatesInSync }.map(\.id) + return Set(keptOff).union(ownersKeptOffSync.subtracting(connections.map(\.id))) + } + ) + } + + var knowsOwners: Bool { + excludedConnectionIds != nil + } + + func includes(_ type: SyncRecordType) -> Bool { + includedTypes.contains(type) + } + + func includes(_ type: SyncRecordType, owner: UUID?) -> Bool { + guard includes(type) else { return false } + guard let owner else { return true } + guard let excludedConnectionIds else { return false } + return !excludedConnectionIds.contains(owner) + } + + func includes(_ tombstone: Tombstone, of type: SyncRecordType) -> Bool { + includes(type, owner: Self.owner(of: tombstone, type: type)) + } + + static func owner(of tombstone: Tombstone, type: SyncRecordType) -> UUID? { + tombstone.owner ?? owner(ofRecordId: tombstone.id, type: type) + } + + static func owner(ofRecordId id: String, type: SyncRecordType) -> UUID? { + switch type { + case .settings: + return FileColumnLayoutPersister.connectionId(ofSyncCategory: id) + case .connection, .group, .tag, .sshProfile, .credentialProfile, + .tableFavorite, .favoriteDatabase, .favorite, .favoriteFolder: + return nil + } + } +} diff --git a/TablePro/Core/Sync/SyncChangeTracker.swift b/TablePro/Core/Sync/SyncChangeTracker.swift index 2faea5c48a..cd06aedaa5 100644 --- a/TablePro/Core/Sync/SyncChangeTracker.swift +++ b/TablePro/Core/Sync/SyncChangeTracker.swift @@ -81,14 +81,55 @@ final class SyncChangeTracker: Sendable { @MainActor func markDeleted(_ type: SyncRecordType, ids: [String]) { + markDeleted(type, ids: ids, owner: nil) + } + + @MainActor + func markDeleted(_ type: SyncRecordType, ids: [String], owner: UUID?) { + guard !isSuppressed, !ids.isEmpty else { return } + metadataStorage.removeDirty(ids, type: type) + metadataStorage.addTombstones(ids, type: type, owner: owner) + recordEdits(type, ids: ids) + Self.logger.trace("Marked deleted: \(type.rawValue) x\(ids.count)") + postChangeNotification() + } + + @MainActor + func markDeleted(_ type: SyncRecordType, idsByOwner: [UUID: Set]) { + let ids = idsByOwner.values.flatMap { $0 } guard !isSuppressed, !ids.isEmpty else { return } metadataStorage.removeDirty(ids, type: type) - metadataStorage.addTombstones(ids, type: type) + metadataStorage.addTombstones( + idsByOwner.flatMap { owner, ownedIds in ownedIds.map { Tombstone(id: $0, owner: owner) } }, + type: type + ) recordEdits(type, ids: ids) Self.logger.trace("Marked deleted: \(type.rawValue) x\(ids.count)") postChangeNotification() } + @MainActor + func discardTombstones(ownedBy owners: Set) { + guard !owners.isEmpty else { return } + metadataStorage.removeTombstones { type, tombstone in + SyncBoundary.owner(of: tombstone, type: type).map(owners.contains) ?? false + } + } + + var ownersKeptOffSync: Set { + metadataStorage.ownersKeptOffSync() + } + + @MainActor + func keepOffSync(owners: Set) { + metadataStorage.keepOffSync(owners: owners) + } + + @MainActor + func releaseOwnersKeptOffSync(_ owners: Set) { + metadataStorage.releaseOwnersKeptOffSync(owners) + } + // MARK: - Query func dirtyRecords(for type: SyncRecordType) -> Set { diff --git a/TablePro/Core/Sync/SyncCoordinator.swift b/TablePro/Core/Sync/SyncCoordinator.swift index 552a80e134..06f53533b5 100644 --- a/TablePro/Core/Sync/SyncCoordinator.swift +++ b/TablePro/Core/Sync/SyncCoordinator.swift @@ -127,7 +127,6 @@ final class SyncCoordinator: ObservableObject { lastSyncDate = Date() metadataStorage.lastSyncDate = lastSyncDate settle(.idle, from: generation) - metadataStorage.pruneTombstones(olderThan: 30) Self.logger.info("Sync completed successfully") } @@ -249,7 +248,7 @@ final class SyncCoordinator: ObservableObject { let connections = services.connectionStorage.loadConnections() changeTracker.markDirty( .connection, - ids: connections.filter { !$0.localOnly }.map { $0.id.uuidString } + ids: connections.filter(\.participatesInSync).map { $0.id.uuidString } ) let groups = services.groupStorage.loadGroups() @@ -382,12 +381,15 @@ final class SyncCoordinator: ObservableObject { private func performPush() async -> PushReport { let snapshot = changeTracker.editSnapshot() - let settings = services.appSettingsStorage.loadSync() + let boundary = syncBoundary(settings: services.appSettingsStorage.loadSync()) let zoneID = await transport.currentZoneID - let batch = await collectPushBatch(snapshot: snapshot, settings: settings, zoneID: zoneID) + let batch = await collectPushBatch(snapshot: snapshot, boundary: boundary, zoneID: zoneID) let deletions = batch.uniqueDeletions - guard !batch.records.isEmpty || !deletions.isEmpty else { return PushReport() } + guard !batch.records.isEmpty || !deletions.isEmpty else { + pruneTombstones(within: boundary) + return PushReport() + } let identities = SyncRecordMapper.identities(for: pushedLocalIds(snapshot), in: zoneID) var outcome: PushOutcome @@ -426,6 +428,7 @@ final class SyncCoordinator: ObservableObject { return PushReport(echoGuard: echoGuard, error: interruption) } guard outcome.hasFailures, let firstFailure = outcome.failures.values.first else { + pruneTombstones(within: boundary) return PushReport(echoGuard: echoGuard) } let rejection = SyncError.pushRejected(count: outcome.failures.count, detail: firstFailure.message) @@ -558,7 +561,7 @@ final class SyncCoordinator: ObservableObject { ) changeTracker.isSuppressed = false - changeTracker.markDeleted(.tableFavorite, ids: Array(effects.tableFavoriteIdsToRetire)) + changeTracker.markDeleted(.tableFavorite, idsByOwner: effects.tableFavoriteIdsToRetire) return !effects.persistenceFailed } @@ -577,6 +580,7 @@ final class SyncCoordinator: ObservableObject { let tagTombstoneIds = Set(metadataStorage.tombstones(for: .tag).map(\.id)) let sshTombstoneIds = Set(metadataStorage.tombstones(for: .sshProfile).map(\.id)) let credentialTombstoneIds = Set(metadataStorage.tombstones(for: .credentialProfile).map(\.id)) + let settingsTombstoneIds = Set(metadataStorage.tombstones(for: .settings).map(\.id)) let tableFavoriteTombstoneIds = Set(metadataStorage.tombstones(for: .tableFavorite).map(\.id)) var tableFavorites: [FavoriteTablesStorage.FavoriteEntry] = [] let databaseFavoriteTombstoneIds = Set(metadataStorage.tombstones(for: .favoriteDatabase).map(\.id)) @@ -613,7 +617,7 @@ final class SyncCoordinator: ObservableObject { persistenceFailed = true } case .settings: - applyRemoteSettings(record) + applyRemoteSettings(record, tombstoneIds: settingsTombstoneIds) case .tableFavorite: if let favorite = remoteTableFavorite(record, tombstoneIds: tableFavoriteTombstoneIds) { tableFavorites.append(favorite) @@ -824,8 +828,9 @@ final class SyncCoordinator: ObservableObject { services.sshProfileStorage.refreshLinkedConnections(with: remoteProfile) } - private func applyRemoteSettings(_ record: CKRecord) { + private func applyRemoteSettings(_ record: CKRecord, tombstoneIds: Set) { guard let category = SyncRecordMapper.settingsCategory(from: record), + !tombstoneIds.contains(category), let data = SyncRecordMapper.settingsData(from: record) else { return } do { diff --git a/TablePro/Models/Connection/DatabaseConnection.swift b/TablePro/Models/Connection/DatabaseConnection.swift index bced2cce19..10ca1821f8 100644 --- a/TablePro/Models/Connection/DatabaseConnection.swift +++ b/TablePro/Models/Connection/DatabaseConnection.swift @@ -383,6 +383,10 @@ extension DatabaseConnection { // MARK: - Device-Local State internal extension DatabaseConnection { + var participatesInSync: Bool { + !localOnly && !isSample + } + func adoptingDeviceLocalState(from local: DatabaseConnection) -> DatabaseConnection { var adopted = self adopted.localOnly = local.localOnly diff --git a/TableProTests/Core/Storage/FavoriteDatabasesStorageTests.swift b/TableProTests/Core/Storage/FavoriteDatabasesStorageTests.swift index 6a471f6350..ef021d007f 100644 --- a/TableProTests/Core/Storage/FavoriteDatabasesStorageTests.swift +++ b/TableProTests/Core/Storage/FavoriteDatabasesStorageTests.swift @@ -4,8 +4,8 @@ // import Foundation -import Testing import TableProSyncTransport +import Testing @testable import TablePro @@ -230,7 +230,7 @@ struct FavoriteDatabasesStorageTests { storage.setFavoriteWithoutSync(entry) #expect(storage.favorites(for: connectionId).first?.environment == .testing) - storage.removeFavoriteWithoutSync(id: FavoriteDatabasesStorage.syncId(for: entry)) + storage.removeFavoritesWithoutSync(ids: [FavoriteDatabasesStorage.syncId(for: entry)]) #expect(storage.favorites(for: connectionId).isEmpty) #expect(metadata.dirtyIds(for: .favoriteDatabase).isEmpty) #expect(metadata.tombstones(for: .favoriteDatabase).isEmpty) diff --git a/TableProTests/Core/Storage/SQLFavoriteStorageTests.swift b/TableProTests/Core/Storage/SQLFavoriteStorageTests.swift index 7c3a140942..a0ff23b1fa 100644 --- a/TableProTests/Core/Storage/SQLFavoriteStorageTests.swift +++ b/TableProTests/Core/Storage/SQLFavoriteStorageTests.swift @@ -89,7 +89,7 @@ struct SQLFavoriteStorageTests { let fav = makeFavorite() _ = await storage.addFavorite(fav) - let deleted = await storage.deleteFavorite(id: fav.id) + let deleted = await storage.deleteFavorites(ids: [fav.id]) != nil #expect(deleted) let fetched = await storage.fetchFavorites() diff --git a/TableProTests/Core/Storage/SQLFavoriteVersionTests.swift b/TableProTests/Core/Storage/SQLFavoriteVersionTests.swift index de726b4e6d..b7b48aa88a 100644 --- a/TableProTests/Core/Storage/SQLFavoriteVersionTests.swift +++ b/TableProTests/Core/Storage/SQLFavoriteVersionTests.swift @@ -104,7 +104,7 @@ struct SQLFavoriteVersionTests { #expect(await storage.updateFavorite(favorite).succeeded) #expect(await storage.fetchVersions(favoriteId: favorite.id).count == 1) - #expect(await storage.deleteFavorite(id: favorite.id)) + #expect(await storage.deleteFavorites(ids: [favorite.id]) != nil) #expect(await storage.fetchVersions(favoriteId: favorite.id).isEmpty) } @@ -135,7 +135,7 @@ struct SQLFavoriteVersionTests { favorite.query = "SELECT new" #expect(await storage.updateFavorite(favorite).succeeded) let old = try #require(await manager.fetchVersions(favoriteId: favorite.id).first) - #expect(await storage.deleteFavorite(id: favorite.id)) + #expect(await storage.deleteFavorites(ids: [favorite.id]) != nil) #expect(await manager.restore(old) == false) } diff --git a/TableProTests/Core/Sync/SyncBoundaryTests.swift b/TableProTests/Core/Sync/SyncBoundaryTests.swift new file mode 100644 index 0000000000..4a6e5e1e16 --- /dev/null +++ b/TableProTests/Core/Sync/SyncBoundaryTests.swift @@ -0,0 +1,111 @@ +import Foundation +@testable import TablePro +import TableProSyncTransport +import Testing + +@MainActor +struct SyncBoundaryTests { + private static let everyCategoryOn = SyncSettings( + enabled: true, + syncConnections: true, + syncGroupsAndTags: true, + syncSettings: true + ) + + private static func connection(localOnly: Bool = false, isSample: Bool = false) -> DatabaseConnection { + var connection = TestFixtures.makeConnection() + connection.localOnly = localOnly + connection.isSample = isSample + return connection + } + + @Test("Every record type is in scope exactly when its category syncs and its type is deployed") + func typeScopeFollowsCategoryAndDeployment() { + var settings = Self.everyCategoryOn + settings.syncTableFavorites = false + let writable = SyncRecordType.verifiedInProduction.subtracting([.favoriteDatabase]) + + let boundary = SyncBoundary(settings: settings, connections: [], writableTypes: writable) + + for type in SyncRecordType.allCases { + let expected = settings.syncs(type) && writable.contains(type) + #expect(boundary.includes(type) == expected, "\(type.rawValue)") + } + #expect(!boundary.includes(.tableFavorite)) + #expect(!boundary.includes(.favoriteDatabase)) + #expect(boundary.includes(.favorite)) + } + + @Test("Local only and sample connections are refused as owners, synced and unknown ones are not") + func ownersKeptOffICloudAreRefused() { + let localOnly = Self.connection(localOnly: true) + let sample = Self.connection(isSample: true) + let synced = Self.connection() + let boundary = SyncBoundary(settings: Self.everyCategoryOn, connections: [localOnly, sample, synced]) + + #expect(boundary.excludedConnectionIds == [localOnly.id, sample.id]) + #expect(!boundary.includes(.tableFavorite, owner: localOnly.id)) + #expect(!boundary.includes(.favorite, owner: sample.id)) + #expect(boundary.includes(.tableFavorite, owner: synced.id)) + #expect(boundary.includes(.favoriteDatabase, owner: UUID())) + #expect(boundary.includes(.favorite, owner: nil)) + } + + @Test("A deleted connection still being purged stays refused, and a connection that exists again follows its own flag") + func ownersKeptOffSyncApplyOnlyToDeletedConnections() { + let deleted = UUID() + let reimported = Self.connection() + let boundary = SyncBoundary( + settings: Self.everyCategoryOn, + connections: [reimported], + ownersKeptOffSync: [deleted, reimported.id] + ) + + #expect(!boundary.includes(.favorite, owner: deleted)) + #expect(boundary.includes(.favorite, owner: reimported.id)) + } + + @Test("An owner in scope is still refused when its record type is not") + func ownerDoesNotOverrideCategory() { + var settings = Self.everyCategoryOn + settings.syncSQLFavorites = false + let boundary = SyncBoundary(settings: settings, connections: [Self.connection()]) + + #expect(!boundary.includes(.favorite, owner: nil)) + #expect(!boundary.includes(.favoriteFolder, owner: UUID())) + } + + @Test("A connection store that cannot be read holds every owned record and lets unowned ones through") + func unreadableStoreHoldsOwnedRecords() { + let boundary = SyncBoundary(settings: Self.everyCategoryOn, connections: nil) + + #expect(!boundary.knowsOwners) + #expect(!boundary.includes(.tableFavorite, owner: UUID())) + #expect(boundary.includes(.favorite, owner: nil)) + #expect(boundary.includes(.tag)) + } + + @Test("A tombstone is held by the owner it was written with") + func tombstoneHeldByItsOwner() { + let localOnly = Self.connection(localOnly: true) + let boundary = SyncBoundary(settings: Self.everyCategoryOn, connections: [localOnly]) + + #expect(!boundary.includes(Tombstone(id: "a", owner: localOnly.id), of: .tableFavorite)) + #expect(boundary.includes(Tombstone(id: "b", owner: UUID()), of: .tableFavorite)) + #expect(boundary.includes(Tombstone(id: "c"), of: .tableFavorite)) + } + + @Test("A column layout tombstone written before owners existed is held by the connection its name carries") + func legacyLayoutTombstoneOwnerComesFromItsName() { + let localOnly = Self.connection(localOnly: true) + let boundary = SyncBoundary(settings: Self.everyCategoryOn, connections: [localOnly]) + let key = ColumnLayoutTableKey( + connectionId: localOnly.id, databaseName: "shop", schemaName: "public", tableName: "orders" + ) + let category = FileColumnLayoutPersister.syncCategory(for: key.storageKey) + + #expect(SyncBoundary.owner(ofRecordId: category, type: .settings) == localOnly.id) + #expect(SyncBoundary.owner(ofRecordId: AppSettingsCategory.editor, type: .settings) == nil) + #expect(!boundary.includes(Tombstone(id: category), of: .settings)) + } +} diff --git a/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift b/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift new file mode 100644 index 0000000000..96b90ecd0b --- /dev/null +++ b/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift @@ -0,0 +1,594 @@ +import CloudKit +import Foundation +@testable import TablePro +import TableProSyncTransport +import Testing + +@MainActor +struct SyncLocalOnlyDependentsTests { + enum KeptOffICloud: CaseIterable { + case localOnly + case sample + } + + @MainActor + private struct Dependents { + let tableFavorite: FavoriteTablesStorage.FavoriteEntry + let databaseFavorite: FavoriteDatabaseEntry + let savedQuery: SQLFavorite + let folder: SQLFavoriteFolder + let layoutKey: ColumnLayoutTableKey + + var tableFavoriteId: String { FavoriteTablesStorage.syncId(for: tableFavorite) } + var databaseFavoriteId: String { FavoriteDatabasesStorage.syncId(for: databaseFavorite) } + var layoutCategory: String { FileColumnLayoutPersister.syncCategory(for: layoutKey.storageKey) } + + var recordIDs: Set { + [ + SyncLocalOnlyDependentsTests.recordID(.tableFavorite, tableFavoriteId), + SyncLocalOnlyDependentsTests.recordID(.favoriteDatabase, databaseFavoriteId), + SyncLocalOnlyDependentsTests.recordID(.favorite, savedQuery.id.uuidString), + SyncLocalOnlyDependentsTests.recordID(.favoriteFolder, folder.id.uuidString), + SyncLocalOnlyDependentsTests.recordID(.settings, layoutCategory) + ] + } + } + + private static let zoneID = SyncTestEnvironment.zoneID + + private let environment: SyncTestEnvironment + + init() throws { + environment = try SyncTestEnvironment(label: "sync-local-only-dependents") + } + + private var tracker: SyncChangeTracker { environment.tracker } + private var metadata: SyncMetadataStorage { environment.metadata } + + private static func recordID(_ type: SyncRecordType, _ id: String) -> CKRecord.ID { + SyncRecordMapper.recordID(type: type, id: id, in: zoneID) + } + + private func addConnection(_ keptOff: KeptOffICloud? = nil) -> DatabaseConnection { + var connection = TestFixtures.makeConnection() + connection.localOnly = keptOff == .localOnly + connection.isSample = keptOff == .sample + environment.connections.addConnection(connection) + return connection + } + + private func layoutKey(_ connectionId: UUID, table: String = "orders") -> ColumnLayoutTableKey { + ColumnLayoutTableKey(connectionId: connectionId, databaseName: "shop", schemaName: "public", tableName: table) + } + + private func saveLayout(_ key: ColumnLayoutTableKey) { + var layout = ColumnLayoutState() + layout.columnWidths = ["id": 80] + environment.columnLayouts.save(layout, for: key) + } + + private func addDependents(of connectionId: UUID) async -> Dependents { + let dependents = Dependents( + tableFavorite: FavoriteTablesStorage.FavoriteEntry( + connectionId: connectionId, database: "shop", schema: "public", name: "orders" + ), + databaseFavorite: FavoriteDatabaseEntry( + connectionId: connectionId, database: "shop", environment: .production + ), + savedQuery: SQLFavorite(name: "Revenue", query: "SELECT 1", connectionId: connectionId), + folder: SQLFavoriteFolder(name: "Reports", connectionId: connectionId), + layoutKey: layoutKey(connectionId) + ) + environment.favoriteTables.addFavorite(name: "orders", schema: "public", database: "shop", connectionId: connectionId) + environment.favoriteDatabases.setFavorite(database: "shop", environment: .production, connectionId: connectionId) + #expect(await environment.favorites.addFavorite(dependents.savedQuery)) + #expect(await environment.favorites.addFolder(dependents.folder)) + saveLayout(dependents.layoutKey) + return dependents + } + + private func expectMarked(_ dependents: Dependents) { + #expect(tracker.dirtyRecords(for: .tableFavorite).contains(dependents.tableFavoriteId)) + #expect(tracker.dirtyRecords(for: .favoriteDatabase).contains(dependents.databaseFavoriteId)) + #expect(tracker.dirtyRecords(for: .favorite).contains(dependents.savedQuery.id.uuidString)) + #expect(tracker.dirtyRecords(for: .favoriteFolder).contains(dependents.folder.id.uuidString)) + #expect(tracker.dirtyRecords(for: .settings).contains(dependents.layoutCategory)) + } + + private func runCycle(_ transport: ScriptedSyncTransport) async -> SyncError? { + await environment.makeCoordinator(transport: transport).runSyncCycle() + } + + private static func isolatedQueryHistory() -> QueryHistoryManager { + QueryHistoryManager( + storage: QueryHistoryStorage( + databaseURL: FileManager.default.temporaryDirectory + .appendingPathComponent("tablepro-tests") + .appendingPathComponent("local-only-purge-\(UUID().uuidString).db"), + removeDatabaseOnDeinit: true + ), + isCapturePaused: { false } + ) + } + + // MARK: - Push + + @Test( + "A connection kept off iCloud keeps its favorites, saved queries, folders and layouts off it, marks held", + arguments: KeptOffICloud.allCases + ) + func keptOffConnectionHoldsItsDependents(_ keptOff: KeptOffICloud) async { + let connection = addConnection(keptOff) + let dependents = await addDependents(of: connection.id) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedRecords.isEmpty) + #expect(await transport.pushedDeletions.isEmpty) + expectMarked(dependents) + } + + @Test("A synced connection's favorites, saved queries, folders and layouts go up and their marks clear") + func syncedConnectionPushesItsDependents() async { + let connection = addConnection() + let dependents = await addDependents(of: connection.id) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(Set(await transport.pushedRecords.map(\.recordID)).isSuperset(of: dependents.recordIDs)) + #expect(tracker.dirtyRecords(for: .tableFavorite).isEmpty) + #expect(tracker.dirtyRecords(for: .favoriteDatabase).isEmpty) + #expect(tracker.dirtyRecords(for: .favorite).isEmpty) + #expect(tracker.dirtyRecords(for: .favoriteFolder).isEmpty) + #expect(tracker.dirtyRecords(for: .settings).isEmpty) + } + + @Test("Deletions of a Local only connection's favorites, saved queries, folders and layouts are held") + func localOnlyDeletionsAreHeld() async { + let connection = addConnection(.localOnly) + let dependents = await addDependents(of: connection.id) + environment.favoriteTables.removeFavorite(name: "orders", schema: "public", database: "shop", connectionId: connection.id) + environment.favoriteDatabases.removeFavorite(database: "shop", connectionId: connection.id) + #expect(await environment.favorites.deleteFavorite(id: dependents.savedQuery.id)) + #expect(await environment.favorites.deleteFolder(id: dependents.folder.id)) + environment.columnLayouts.clear(for: dependents.layoutKey) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite) == [dependents.tableFavoriteId]) + #expect(tracker.tombstonedIds(for: .favoriteDatabase) == [dependents.databaseFavoriteId]) + #expect(tracker.tombstonedIds(for: .favorite) == [dependents.savedQuery.id.uuidString]) + #expect(tracker.tombstonedIds(for: .favoriteFolder) == [dependents.folder.id.uuidString]) + #expect(tracker.tombstonedIds(for: .settings) == [dependents.layoutCategory]) + } + + @Test("Putting a connection back in sync sends the edits and deletions held while it was Local only") + func reincludedConnectionReleasesHeldWork() async { + let connection = addConnection(.localOnly) + let kept = await addDependents(of: connection.id) + environment.favoriteTables.addFavorite(name: "gone", schema: "public", database: "shop", connectionId: connection.id) + environment.favoriteTables.removeFavorite(name: "gone", schema: "public", database: "shop", connectionId: connection.id) + let removedId = FavoriteTablesStorage.syncId(for: FavoriteTablesStorage.FavoriteEntry( + connectionId: connection.id, database: "shop", schema: "public", name: "gone" + )) + #expect(await runCycle(ScriptedSyncTransport(zoneID: Self.zoneID)) == nil) + #expect(environment.connections.mutateConnections(ids: [connection.id]) { $0.localOnly = false }) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(Set(await transport.pushedRecords.map(\.recordID)).isSuperset(of: kept.recordIDs)) + #expect(await transport.pushedDeletions == [Self.recordID(.tableFavorite, removedId)]) + #expect(tracker.tombstonedIds(for: .tableFavorite).isEmpty) + } + + @Test("A deletion recorded before deletions had owners still goes up") + func legacyOwnerlessTombstoneStillPushes() async { + metadata.addTombstone("legacy", type: .tableFavorite) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedDeletions == [Self.recordID(.tableFavorite, "legacy")]) + } + + @Test("An unreadable connection list holds every record that belongs to a connection and sends the rest") + func unreadableConnectionStoreHoldsOwnedRecords() async throws { + let connection = addConnection() + environment.favoriteTables.addFavorite(name: "orders", schema: nil, database: "shop", connectionId: connection.id) + let global = SQLFavorite(name: "Everywhere", query: "SELECT 1") + #expect(await environment.favorites.addFavorite(global)) + metadata.addTombstones(["owned"], type: .favoriteDatabase, owner: connection.id) + try Data("not json".utf8).write(to: environment.directory.appendingPathComponent("connections.json")) + environment.connections.invalidateCache() + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedRecords.map(\.recordID) == [Self.recordID(.favorite, global.id.uuidString)]) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.dirtyRecords(for: .tableFavorite).count == 1) + #expect(tracker.tombstonedIds(for: .favoriteDatabase) == ["owned"]) + } + + @Test("A connection edit still goes up when the push could not tell which connections are kept off iCloud") + func connectionEditSurvivesUnknownOwners() async { + let connection = addConnection() + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + let boundary = SyncBoundary(includedTypes: Set(SyncRecordType.allCases), excludedConnectionIds: nil) + + let batch = await coordinator.collectPushBatch( + snapshot: tracker.editSnapshot(), boundary: boundary, zoneID: Self.zoneID + ) + + #expect(batch.records.map(\.recordID) == [Self.recordID(.connection, connection.id.uuidString)]) + #expect(tracker.dirtyRecords(for: .connection) == [connection.id.uuidString]) + } + + // MARK: - Column layout deletions + + @Test("A column layout cleared on a synced connection sends its deletion") + func clearedLayoutSendsItsDeletion() async { + let connection = addConnection() + let key = layoutKey(connection.id) + saveLayout(key) + #expect(await runCycle(ScriptedSyncTransport(zoneID: Self.zoneID)) == nil) + environment.columnLayouts.clear(for: key) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + let category = FileColumnLayoutPersister.syncCategory(for: key.storageKey) + #expect(failure == nil) + #expect(await transport.pushedDeletions == [Self.recordID(.settings, category)]) + #expect(tracker.tombstonedIds(for: .settings).isEmpty) + } + + @Test("A layout deletion left over for a layout this Mac still holds is dropped rather than sent") + func staleLayoutTombstoneIsDropped() async { + let connection = addConnection() + let key = layoutKey(connection.id) + saveLayout(key) + #expect(await runCycle(ScriptedSyncTransport(zoneID: Self.zoneID)) == nil) + let category = FileColumnLayoutPersister.syncCategory(for: key.storageKey) + metadata.addTombstone(category, type: .settings) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.tombstonedIds(for: .settings).isEmpty) + #expect(environment.columnLayouts.load(for: key) != nil) + } + + @Test("A layout this Mac is deleting is not brought back by a pull before the deletion goes up") + func pulledLayoutWaitingOnItsDeletionStaysDeleted() async throws { + let connection = addConnection() + let key = layoutKey(connection.id) + saveLayout(key) + let category = FileColumnLayoutPersister.syncCategory(for: key.storageKey) + let remote = SyncRecordMapper.toCKRecord( + category: category, + settingsData: Data(#"{"columnWidths":{"id":120}}"#.utf8), + in: Self.zoneID + ) + environment.columnLayouts.clear(for: key) + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + + let acknowledged = await coordinator.applyPullResult( + PullResult(changedRecords: [remote], deletedRecordIDs: [], newToken: nil) + ) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + let failure = await runCycle(transport) + + #expect(acknowledged) + #expect(failure == nil) + #expect(environment.columnLayouts.load(for: key) == nil) + #expect(await transport.pushedDeletions == [Self.recordID(.settings, category)]) + } + + @Test("A layout deletion from another Mac that cannot be written here is not acknowledged") + func unwritableLayoutDeletionIsNotAcknowledged() async throws { + let connection = addConnection() + let removed = layoutKey(connection.id) + let kept = layoutKey(connection.id, table: "customers") + saveLayout(removed) + saveLayout(kept) + let directory = environment.directory.appendingPathComponent("ColumnLayout", isDirectory: true) + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: directory.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: directory.path) } + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + + let acknowledged = await coordinator.applyPullResult(PullResult( + changedRecords: [], + deletedRecordIDs: [Self.recordID(.settings, FileColumnLayoutPersister.syncCategory(for: removed.storageKey))], + newToken: nil + )) + + #expect(!acknowledged) + #expect(environment.columnLayouts.load(for: removed) != nil) + #expect(tracker.dirtyRecords(for: .settings).contains(FileColumnLayoutPersister.syncCategory(for: removed.storageKey))) + } + + @Test("A layout clear that cannot be written leaves the layout and records no deletion") + func unwritableLayoutClearRecordsNoDeletion() throws { + let connection = addConnection() + let cleared = layoutKey(connection.id) + saveLayout(cleared) + saveLayout(layoutKey(connection.id, table: "customers")) + let directory = environment.directory.appendingPathComponent("ColumnLayout", isDirectory: true) + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: directory.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: directory.path) } + + environment.columnLayouts.clear(for: cleared) + + #expect(environment.columnLayouts.load(for: cleared) != nil) + #expect(tracker.tombstonedIds(for: .settings).isEmpty) + } + + // MARK: - Deleting the connection + + @Test("Deleting a Local only connection removes its dependents without a trace in iCloud sync") + func localOnlyPurgeLeavesNoTombstones() async throws { + let connection = addConnection(.localOnly) + let dependents = await addDependents(of: connection.id) + environment.favoriteTables.addFavorite(name: "gone", schema: nil, database: "shop", connectionId: connection.id) + environment.favoriteTables.removeFavorite(name: "gone", schema: nil, database: "shop", connectionId: connection.id) + #expect(tracker.tombstonedIds(for: .tableFavorite).count == 1) + #expect(environment.connections.saveConnections([])) + let history = Self.isolatedQueryHistory() + + ConnectionLocalState.purge( + connectionIds: [connection.id], + origin: .localOnly, + tableScopedStores: [environment.columnLayouts], + favoriteTables: environment.favoriteTables, + favoriteDatabases: environment.favoriteDatabases, + sqlFavorites: environment.favorites, + queryHistory: history, + syncTracker: tracker + ) + await ConnectionLocalState.purgeAsyncStores( + [connection.id], origin: .localOnly, sqlFavorites: environment.favorites, queryHistory: history, + syncTracker: tracker + ) + let transport = ScriptedSyncTransport(zoneID: Self.zoneID) + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedRecords.isEmpty) + #expect(await transport.pushedDeletions.isEmpty) + #expect(await environment.favorites.fetchFavorite(id: dependents.savedQuery.id) == nil) + #expect(environment.favoriteTables.favorites(for: connection.id).isEmpty) + #expect(tracker.ownersKeptOffSync.isEmpty) + for type in SyncRecordType.allCases { + #expect(tracker.tombstonedIds(for: type).isEmpty, "\(type.rawValue)") + #expect(tracker.dirtyRecords(for: type).isEmpty, "\(type.rawValue)") + } + } + + @Test("A deleted Local only connection stays out of iCloud until its saved queries are gone") + func deletedLocalOnlyOwnerStaysExcludedUntilItsSavedQueriesAreGone() async { + let connection = addConnection(.localOnly) + let savedQuery = SQLFavorite(name: "Revenue", query: "SELECT 1", connectionId: connection.id) + #expect(await environment.favorites.addFavorite(savedQuery)) + #expect(environment.connections.saveConnections([])) + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + let history = Self.isolatedQueryHistory() + + ConnectionLocalState.purge( + connectionIds: [connection.id], + origin: .localOnly, + tableScopedStores: [], + favoriteTables: environment.favoriteTables, + favoriteDatabases: environment.favoriteDatabases, + sqlFavorites: environment.favorites, + queryHistory: history, + syncTracker: tracker + ) + + #expect(tracker.ownersKeptOffSync == [connection.id]) + #expect(!coordinator.syncBoundary(settings: .default).includes(.favorite, owner: connection.id)) + + await ConnectionLocalState.purgeAsyncStores( + [connection.id], origin: .localOnly, sqlFavorites: environment.favorites, queryHistory: history, + syncTracker: tracker + ) + + #expect(await environment.favorites.fetchFavorite(id: savedQuery.id) == nil) + #expect(tracker.ownersKeptOffSync.isEmpty) + } + + @Test("Deleting a Local only connection forgets the deletions held for it, and a synced one keeps them") + func connectionDeleteDiscardsOnlyALocalOnlyOwnersTombstones() { + let localOnly = addConnection(.localOnly) + let synced = addConnection() + metadata.addTombstones(["held"], type: .tableFavorite, owner: localOnly.id) + metadata.addTombstones(["pending"], type: .tableFavorite, owner: synced.id) + metadata.addTombstone( + FileColumnLayoutPersister.syncCategory(for: layoutKey(localOnly.id).storageKey), + type: .settings + ) + + #expect(environment.connections.deleteConnection(localOnly)) + #expect(tracker.tombstonedIds(for: .tableFavorite) == ["pending"]) + #expect(tracker.tombstonedIds(for: .settings).isEmpty) + #expect(tracker.tombstonedIds(for: .connection).isEmpty) + + #expect(environment.connections.deleteConnections([synced])) + #expect(tracker.tombstonedIds(for: .tableFavorite) == ["pending"]) + #expect(tracker.tombstonedIds(for: .connection) == [synced.id.uuidString]) + } + + // MARK: - Pruning + + @Test("Pruning keeps a month-old deletion held for a Local only connection and drops a pushable one") + func pruningKeepsHeldTombstones() throws { + let localOnly = addConnection(.localOnly) + let synced = addConnection() + let fortyDaysAgo = Date(timeIntervalSinceNow: -60 * 60 * 24 * 40) + let tombstones = [ + Tombstone(id: "held", deletedAt: fortyDaysAgo, owner: localOnly.id), + Tombstone(id: "expired", deletedAt: fortyDaysAgo, owner: synced.id), + Tombstone(id: "recent", deletedAt: Date(), owner: synced.id) + ] + metadata.userDefaults.set( + try JSONEncoder().encode(tombstones), + forKey: "com.TablePro.sync.tombstones.\(SyncRecordType.tableFavorite.rawValue)" + ) + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + + coordinator.pruneTombstones(within: coordinator.syncBoundary(settings: SyncSettings.default)) + + #expect(tracker.tombstonedIds(for: .tableFavorite) == ["held", "recent"]) + } + + @Test("A deletion held when the push ran is not pruned by a connection put back in sync during the cycle") + func heldTombstoneSurvivesAScopeChangeMidCycle() async throws { + let connection = addConnection(.localOnly) + let fortyDaysAgo = Date(timeIntervalSinceNow: -60 * 60 * 24 * 40) + metadata.userDefaults.set( + try JSONEncoder().encode([Tombstone(id: "held", deletedAt: fortyDaysAgo, owner: connection.id)]), + forKey: "com.TablePro.sync.tombstones.\(SyncRecordType.tableFavorite.rawValue)" + ) + let global = SQLFavorite(name: "Everywhere", query: "SELECT 1") + #expect(await environment.favorites.addFavorite(global)) + let connections = environment.connections + let transport = ScriptedSyncTransport( + zoneID: Self.zoneID, + duringPush: { _ = connections.mutateConnections(ids: [connection.id]) { $0.localOnly = false } } + ) + + let failure = await runCycle(transport) + + #expect(failure == nil) + #expect(await transport.pushedDeletions.isEmpty) + #expect(tracker.tombstonedIds(for: .tableFavorite) == ["held"]) + } + + // MARK: - Remote deletions + + @Test("A database favorite removed on another Mac is removed here and takes its mark with it") + func remoteDatabaseFavoriteDeletionIsApplied() async { + let connection = addConnection() + environment.favoriteDatabases.setFavorite(database: "shop", environment: .production, connectionId: connection.id) + environment.favoriteDatabases.setFavorite(database: "kept", environment: .testing, connectionId: connection.id) + let removed = FavoriteDatabaseEntry(connectionId: connection.id, database: "shop", environment: .production) + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + + let acknowledged = await coordinator.applyPullResult(PullResult( + changedRecords: [], + deletedRecordIDs: [Self.recordID(.favoriteDatabase, FavoriteDatabasesStorage.syncId(for: removed))], + newToken: nil + )) + + #expect(acknowledged) + #expect(environment.favoriteDatabases.favorites(for: connection.id).map(\.database) == ["kept"]) + #expect(!tracker.dirtyRecords(for: .favoriteDatabase).contains(FavoriteDatabasesStorage.syncId(for: removed))) + #expect(tracker.tombstonedIds(for: .favoriteDatabase).isEmpty) + } + + @Test("Column layouts removed on another Mac are removed here, a digest-named one included") + func remoteColumnLayoutDeletionsAreApplied() async { + let connection = addConnection() + let short = layoutKey(connection.id) + let long = layoutKey(connection.id, table: String(repeating: "t", count: 300)) + let kept = layoutKey(connection.id, table: "customers") + [short, long, kept].forEach(saveLayout) + let longRecordID = Self.recordID(.settings, FileColumnLayoutPersister.syncCategory(for: long.storageKey)) + #expect(longRecordID.recordName.contains(SyncRecordName.digestPrefix)) + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + + let acknowledged = await coordinator.applyPullResult(PullResult( + changedRecords: [], + deletedRecordIDs: [ + Self.recordID(.settings, FileColumnLayoutPersister.syncCategory(for: short.storageKey)), + longRecordID + ], + newToken: nil + )) + + #expect(acknowledged) + #expect(environment.columnLayouts.load(for: short) == nil) + #expect(environment.columnLayouts.load(for: long) == nil) + #expect(environment.columnLayouts.load(for: kept) != nil) + #expect(tracker.dirtyRecords(for: .settings) == [FileColumnLayoutPersister.syncCategory(for: kept.storageKey)]) + #expect(tracker.tombstonedIds(for: .settings).isEmpty) + } + + @Test("Database favorite and column layout deletions from another Mac wait while their category is off") + func remoteDeletionsFollowTheirCategory() async { + let connection = addConnection() + environment.favoriteDatabases.setFavorite(database: "shop", environment: .production, connectionId: connection.id) + let key = layoutKey(connection.id) + saveLayout(key) + var settings = SyncSettings.default + settings.syncDatabaseFavorites = false + settings.syncSettings = false + AppSettingsStorage(userDefaults: environment.defaults).saveSync(settings) + let entry = FavoriteDatabaseEntry(connectionId: connection.id, database: "shop", environment: .production) + let coordinator = environment.makeCoordinator(transport: ScriptedSyncTransport(zoneID: Self.zoneID)) + + let acknowledged = await coordinator.applyPullResult(PullResult( + changedRecords: [], + deletedRecordIDs: [ + Self.recordID(.favoriteDatabase, FavoriteDatabasesStorage.syncId(for: entry)), + Self.recordID(.settings, FileColumnLayoutPersister.syncCategory(for: key.storageKey)) + ], + newToken: nil + )) + + #expect(acknowledged) + #expect(environment.favoriteDatabases.favorites(for: connection.id).count == 1) + #expect(environment.columnLayouts.load(for: key) != nil) + } + + // MARK: - Owners + + @Test("Every store that deletes something belonging to a connection records the connection as its owner") + func dependentStoresRecordTheirOwner() async { + let connection = addConnection() + let dependents = await addDependents(of: connection.id) + let second = SQLFavorite(name: "Second", query: "SELECT 2", connectionId: connection.id) + let global = SQLFavorite(name: "Global", query: "SELECT 3") + #expect(await environment.favorites.addFavorite(second)) + #expect(await environment.favorites.addFavorite(global)) + environment.favoriteTables.addFavorite(name: "a|b", schema: "public", database: "shop", connectionId: connection.id) + let piped = FavoriteTablesStorage.FavoriteEntry( + connectionId: connection.id, database: "shop", schema: "public", name: "a|b" + ) + + environment.favoriteTables.removeFavorites(for: connection.id) + environment.favoriteDatabases.removeFavorites(for: connection.id) + #expect(await environment.favorites.deleteFavorite(id: dependents.savedQuery.id)) + await environment.favorites.deleteFavorites(ids: [second.id, global.id]) + #expect(await environment.favorites.deleteFolder(id: dependents.folder.id)) + environment.columnLayouts.clear(for: dependents.layoutKey) + + let owners = { (type: SyncRecordType) in + Dictionary(uniqueKeysWithValues: metadata.tombstones(for: type).map { ($0.id, $0.owner) }) + } + var tableFavoriteIds = [dependents.tableFavoriteId, FavoriteTablesStorage.syncId(for: piped)] + tableFavoriteIds += [FavoriteTablesStorage.legacyAlias(of: piped)].compactMap { $0 } + #expect(owners(.tableFavorite) == Dictionary(uniqueKeysWithValues: tableFavoriteIds.map { ($0, connection.id) })) + #expect(owners(.favoriteDatabase) == [dependents.databaseFavoriteId: connection.id]) + #expect(owners(.favorite) == [ + dependents.savedQuery.id.uuidString: connection.id, + second.id.uuidString: connection.id, + global.id.uuidString: UUID?.none + ]) + #expect(owners(.favoriteFolder) == [dependents.folder.id.uuidString: connection.id]) + #expect(owners(.settings) == [dependents.layoutCategory: connection.id]) + } +} diff --git a/TableProTests/Core/Sync/SyncPendingDeletionsTests.swift b/TableProTests/Core/Sync/SyncPendingDeletionsTests.swift index 089874663a..8563c7743e 100644 --- a/TableProTests/Core/Sync/SyncPendingDeletionsTests.swift +++ b/TableProTests/Core/Sync/SyncPendingDeletionsTests.swift @@ -131,6 +131,22 @@ struct SyncPendingDeletionsTests { #expect(on.sqlFolders == [Self.uuid]) } + @Test("A database favorite or column layout deleted on another device is withheld while its category is off") + func databaseFavoriteAndLayoutDeletionsFollowTheirSwitches() { + let layoutCategory = "columnLayout.\(Self.uuid.uuidString).shop.public.orders" + let deletions = Self.deletion(of: .favoriteDatabase, id: Self.tableFavoriteId) + + Self.deletion(of: .settings, id: layoutCategory) + let off = SyncPendingDeletions.parse(deletions, settings: Self.settings { + $0.syncDatabaseFavorites = false + $0.syncSettings = false + }) + let on = SyncPendingDeletions.parse(deletions, settings: Self.everyCategoryOn) + + #expect(off == SyncPendingDeletions()) + #expect(on.databaseFavorites == [Self.tableFavoriteId]) + #expect(on.settingsRecordNames == [SyncRecordType.settings.recordName(for: layoutCategory)]) + } + @Test("A category switched off withholds its own deletions and no other") func switchedOffCategoryLeavesOthersApplied() { let pending = SyncPendingDeletions.parse( diff --git a/docs/features/icloud-sync.mdx b/docs/features/icloud-sync.mdx index aa6f9051ce..4c88444d7c 100644 --- a/docs/features/icloud-sync.mdx +++ b/docs/features/icloud-sync.mdx @@ -33,7 +33,9 @@ A sync runs at launch, when you switch back to the app, and 2 seconds after you ## Keeping a connection off iCloud -A localhost or throwaway database is rarely worth a round trip. Mark it **Local only** in the connection form's **Advanced** pane, or right-click it and choose **Exclude from iCloud Sync**. They show a struck-through cloud icon in the connection list, and the flag survives duplicating and exporting. +A localhost or throwaway database is rarely worth a round trip. Mark it **Local only** in the connection form's **Advanced** pane, or right-click it and choose **Exclude from iCloud Sync**. It shows a struck-through cloud icon in the connection list, and the flag survives duplicating and exporting. + +What belongs to the connection stays on this Mac with it: its table and database favorites, the saved queries and folders scoped to it, and its column widths, order, and hidden columns. Edits and removals you make to them wait here while the connection is Local only, and the first sync after you turn it off sends them. Deleting a Local only connection removes those items from this Mac and from nowhere else. ## Checking that it worked