fix(connections): finish the drop and delete lifecycle the catalog adoption started

CHANGELOG.md

@@ -45,6 +45,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- A column resized moments before its table was dropped saving the layout back over the clear.
+- A connection left pointing at a database that was dropped.
 - A dropped table's saved filters, column layout, highlight rules, value formats and label columns coming back on a table recreated with its name.
 - A dropped database or schema leaving every one of its tables' saved settings behind.
 - Favorites rows left behind by a dropped table, schema or database.
@@ -220,6 +222,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security
 
+- SQL favorites deleted by another Mac's sync tombstoned again from this one, pushing the deletion back at it.
 - Query history kept for a connection deleted on another Mac, including the statements' own text and its literals.
 - A locked launch on iPhone and iPad connecting to the last session, and asking to trust a host key, before Face ID was answered.
 - Code inside a plugin bundle, and its resource envelope, were not verified before the bundle was loaded.

TablePro/Core/Services/Query/CatalogEditAdoption.swift

@@ -42,10 +42,19 @@ struct LoadedBrowseCatalog: Sendable, Equatable {
 struct CatalogEditAdoption {
     private let databaseManager: DatabaseManager
     private let schemaService: SchemaService
+    private let connectionStorage: ConnectionStorage
+    private let appSettings: AppSettingsStorage
 
-    init(databaseManager: DatabaseManager = .shared, schemaService: SchemaService = .shared) {
+    init(
+        databaseManager: DatabaseManager = .shared,
+        schemaService: SchemaService = .shared,
+        connectionStorage: ConnectionStorage = .shared,
+        appSettings: AppSettingsStorage = .shared
+    ) {
         self.databaseManager = databaseManager
         self.schemaService = schemaService
+        self.connectionStorage = connectionStorage
+        self.appSettings = appSettings
     }
 
     /// Where the object lives. A reference without a database means the one being browsed, and the
@@ -157,6 +166,7 @@ struct CatalogEditAdoption {
         }
         guard container.kind == .database else { return }
         FavoriteDatabasesStorage.shared.removeFavorite(database: database, connectionId: connectionId)
+        clearSavedConnectionDatabase(named: database, connectionId: connectionId)
         sidebarState.clearRecentTables(inDatabase: database)
         var selected = sidebarState.databaseFilterSelected
         guard selected.remove(database) != nil else { return }
@@ -276,11 +286,31 @@ struct CatalogEditAdoption {
 
     /// The saved default is what a reconnect and Reopen Last Session both use, so a database renamed
     /// out from under it leaves the connection opening onto nothing.
-    private func retargetSavedConnectionDatabase(from oldName: String, to newName: String, connectionId: UUID) {
-        guard var saved = ConnectionStorage.shared.loadConnections().first(where: { $0.id == connectionId }),
+    internal func retargetSavedConnectionDatabase(from oldName: String, to newName: String, connectionId: UUID) {
+        guard var saved = connectionStorage.loadConnections().first(where: { $0.id == connectionId }),
               saved.database == oldName else { return }
         saved.database = newName
-        ConnectionStorage.shared.updateConnection(saved)
+        connectionStorage.updateConnection(saved)
+    }
+
+    /// A rename has a new name to point the saved default at. A drop has none, so it is emptied,
+    /// along with the last database the session remembered.
+    ///
+    /// Both, because `selectDatabaseFromLastSession` fires precisely when the saved default is
+    /// empty: emptying one and leaving the other would turn that action on and point it at the
+    /// database that was just dropped, so every later connect would try to switch to it and fail.
+    ///
+    /// Not for a type that requires a value. Emptying is the repair for an engine that accepts a
+    /// blank database, which is what the form already allows there, and MySQL connects with no
+    /// default while MongoDB picks one. A type whose form refuses to save without a value would be
+    /// left failing its own validation with nothing on screen saying why.
+    internal func clearSavedConnectionDatabase(named database: String, connectionId: UUID) {
+        guard var saved = connectionStorage.loadConnections().first(where: { $0.id == connectionId }),
+              saved.database == database,
+              !ConnectionDatabaseRequirement.requiresValue(for: saved.type) else { return }
+        saved.database = ""
+        connectionStorage.updateConnection(saved)
+        appSettings.saveLastDatabase(nil, for: connectionId)
     }
 
     private func retargetBrowseCursor(_ connection: DatabaseConnection, from oldName: String, to newName: String) {

TablePro/Core/Storage/ColumnLayoutPersister.swift

@@ -209,14 +209,21 @@ final class FileColumnLayoutPersister: ColumnLayoutPersisting, TableScopedSettin
         syncTracker.markDeleted(.settings, ids: dropping.map(Self.syncCategory(for:)))
     }
 
-    func purgeConnections(_ connectionIds: Set<UUID>) {
-        var deletedCategories: [String] = []
+    func purgeConnections(_ connectionIds: Set<UUID>, leavesTombstones: Bool) {
+        var categories: [String] = []
         for connectionId in connectionIds {
-            deletedCategories += loadEntries(for: connectionId).keys.map(Self.syncCategory(for:))
+            categories += loadEntries(for: connectionId).keys.map(Self.syncCategory(for:))
             cache[connectionId] = [:]
             removeFile(for: connectionId)
         }
-        syncTracker.markDeleted(.settings, ids: deletedCategories)
+        /// The dirty marks go either way. A tombstone from a remote delete would push the sender's
+        /// 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)
+        } else {
+            syncTracker.discardDirty(.settings, ids: categories)
+        }
     }
 
     func clear(for key: ColumnLayoutTableKey) {

TablePro/Core/Storage/ConnectionLocalState.swift

@@ -45,12 +45,16 @@ internal enum ConnectionLocalState {
         }
 
         for store in tableScopedStores {
-            store.purgeConnections(connectionIds)
+            store.purgeConnections(connectionIds, leavesTombstones: origin == .local)
         }
         DatabaseTreeFilterStorage.shared.removeFilters(for: connectionIds)
         RecentlyClosedTabStore.shared.removeEntries(for: connectionIds)
         WorkspaceRailOrderStore.shared.removeEntries(for: connectionIds)
-        Task { await purgeAsyncStores(connectionIds, sqlFavorites: sqlFavorites, queryHistory: queryHistory) }
+        Task {
+            await purgeAsyncStores(
+                connectionIds, origin: origin, sqlFavorites: sqlFavorites, queryHistory: queryHistory
+            )
+        }
     }
 
     /// The two stores that can only be reached with `await`, so `purge` fires them and does not
@@ -61,19 +65,27 @@ internal enum ConnectionLocalState {
     ///
     /// Separate from `purge` so a test can await what `purge` cannot.
     ///
-    /// `origin` does not reach here, and the two stores differ on why. Query history is device-local
-    /// and never synced, so a remote delete should forget this device's copy and has no tombstone to
-    /// push back. SQL favorites are synced and `removeFavoritesAndFolders` tombstones every record it
-    /// removes, so a remote delete does push one back at the device that sent it. That predates this
-    /// helper and is carried unchanged rather than fixed here, because the without-sync counterpart
-    /// `purgeFavorites` uses for the table favorites does not exist for these.
+    /// `origin` splits the SQL favorites the way `purgeFavorites` splits the table ones, and for the
+    /// same reason. `SyncCoordinator.applyRemoteChanges` suppresses the change tracker only for the
+    /// length of its own synchronous body, and this runs from a `Task` that starts after that body
+    /// has returned and the suppression has been reset, so a remote delete really did write
+    /// tombstones and push the sender's own deletion back at it.
+    ///
+    /// Query history takes no origin: it is device-local and never synced, so a remote delete
+    /// should forget this device's copy and has no tombstone to leave either way.
     internal static func purgeAsyncStores(
         _ connectionIds: Set<UUID>,
+        origin: Origin,
         sqlFavorites: SQLFavoriteManager = .shared,
         queryHistory: QueryHistoryManager = .shared
     ) async {
         for connectionId in connectionIds {
-            await sqlFavorites.removeFavoritesAndFolders(for: connectionId)
+            switch origin {
+            case .local:
+                await sqlFavorites.removeFavoritesAndFolders(for: connectionId)
+            case .remote:
+                await sqlFavorites.removeFavoritesAndFoldersWithoutSync(for: connectionId)
+            }
             if await !queryHistory.deleteEverything(forConnection: connectionId) {
                 logger.error(
                     "Query history for a deleted connection could not be cleared: \(connectionId, privacy: .public)"

TablePro/Core/Storage/FilterSettingsStorage.swift

@@ -547,7 +547,7 @@ final class FilterSettingsStorage: TableScopedSettingsStore {
         ) + Self.browseKeySuffix
     }
 
-    func purgeConnections(_ connectionIds: Set<UUID>) {
+    func purgeConnections(_ connectionIds: Set<UUID>, leavesTombstones: Bool) {
         guard !connectionIds.isEmpty else { return }
 
         let encodedPrefixes = connectionIds.map { TableScope.storagePrefix(connectionId: $0) }

TablePro/Core/Storage/ForeignKeyLabelColumnStore.swift

@@ -69,7 +69,7 @@ internal final class ForeignKeyLabelColumnStore: TableScopedSettingsStore {
         )
     }
 
-    func purgeConnections(_ connectionIds: Set<UUID>) {
+    func purgeConnections(_ connectionIds: Set<UUID>, leavesTombstones: Bool) {
         for connectionId in connectionIds {
             store.removeValues(withPrefix: Self.keyPrefix + TableScope.storagePrefix(connectionId: connectionId))
         }

TablePro/Core/Storage/HighlightRuleStorage.swift

@@ -94,7 +94,7 @@ final class HighlightRuleStorage: ObservableObject, TableScopedSettingsStore {
         commit(entries, for: connectionId)
     }
 
-    func purgeConnections(_ connectionIds: Set<UUID>) {
+    func purgeConnections(_ connectionIds: Set<UUID>, leavesTombstones: Bool) {
         guard !connectionIds.isEmpty else { return }
         for connectionId in connectionIds {
             cache[connectionId] = [:]

TablePro/Core/Storage/SQLFavoriteManager.swift

@@ -79,6 +79,17 @@ internal final class SQLFavoriteManager: @unchecked Sendable {
         postUpdateNotification(connectionId: nil)
     }
 
+    /// 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 {
+        let removed = await storage.deleteFavoritesAndFolders(connectionId: connectionId)
+        guard !removed.isEmpty else { return }
+        syncTracker.discardDirty(.favorite, ids: removed.favorites.map(\.uuidString))
+        syncTracker.discardDirty(.favoriteFolder, ids: removed.folders.map(\.uuidString))
+        postUpdateNotification(connectionId: nil)
+    }
+
     func pruneOrphaned(activeConnectionIds: Set<UUID>) async {
         await storage.pruneOrphaned(retaining: activeConnectionIds)
     }

TablePro/Core/Storage/TableScopedSettingsStore.swift

@@ -26,7 +26,10 @@ internal protocol TableScopedSettingsStore: AnyObject {
     /// disk and could never be named here. A nil schema means the whole database.
     func dropContainer(connectionId: UUID, database: String, schema: String?)
 
-    func purgeConnections(_ connectionIds: Set<UUID>)
+    /// Forgets everything these connections saved. `leavesTombstones` is false when another device
+    /// did the deleting: a synced store must not mark its records deleted there, or it pushes the
+    /// sender's own deletion back at it.
+    func purgeConnections(_ connectionIds: Set<UUID>, leavesTombstones: Bool)
 }
 
 @MainActor

TablePro/Core/Storage/ValueDisplayFormatStorage.swift

@@ -76,7 +76,7 @@ internal final class ValueDisplayFormatStorage: TableScopedSettingsStore {
         )
     }
 
-    func purgeConnections(_ connectionIds: Set<UUID>) {
+    func purgeConnections(_ connectionIds: Set<UUID>, leavesTombstones: Bool) {
         for connectionId in connectionIds {
             store.removeValues(withPrefix: Self.keyPrefix + TableScope.storagePrefix(connectionId: connectionId))
             store.removeValues(withPrefix: Self.legacyKeyPrefix(for: connectionId))

TablePro/Core/Sync/SyncChangeTracker.swift

@@ -61,6 +61,16 @@ final class SyncChangeTracker: Sendable {
         postChangeNotification()
     }
 
+    /// Forgets that records were waiting to be pushed, without tombstoning them.
+    ///
+    /// For records another device already deleted: a tombstone would send its own deletion back at
+    /// it, but leaving the dirty ids behind is not free either. The next push looks for records
+    /// that are gone, skips them, and never drains the entries.
+    func discardDirty(_ type: SyncRecordType, ids: [String]) {
+        guard !ids.isEmpty else { return }
+        metadataStorage.removeDirty(ids, type: type)
+    }
+
     func markDeleted(_ type: SyncRecordType, ids: [String]) {
         guard !isSuppressed, !ids.isEmpty else { return }
         metadataStorage.removeDirty(ids, type: type)

TablePro/Models/Connection/ConnectionDatabaseRequirement.swift

@@ -0,0 +1,37 @@
+//
+//  ConnectionDatabaseRequirement.swift
+//  TablePro
+//
+
+import Foundation
+import TableProPluginKit
+
+/// Whether a connection of this type has to carry a database name to be usable.
+///
+/// One owner, because two answers drift. The connection form refuses to save without a value when
+/// this is true, and the catalog adoption refuses to empty the field for the same reason: a
+/// dropped database leaves nothing to put there, and a type that requires one would be left
+/// failing its own validation with nothing on screen saying why.
+@MainActor
+internal enum ConnectionDatabaseRequirement {
+    /// Whether the form renders the built-in Database field at all. A driver opts out through
+    /// `hidesBuiltInDatabase` when it names its container some other way, or has none.
+    internal static func showsBuiltInField(for type: DatabaseType) -> Bool {
+        let hidden = PluginMetadataRegistry.shared.snapshot(for: type)?.connection.hidesBuiltInDatabase ?? false
+        switch PluginManager.shared.connectionMode(for: type) {
+        case .fileBased:
+            return false
+        case .apiOnly:
+            return PluginManager.shared.supportsDatabaseSwitching(for: type) && !hidden
+        default:
+            return !hidden
+        }
+    }
+
+    /// Never require a value the form does not render. A file-based connection stores its path in
+    /// `database` and renders it as the Database File field.
+    internal static func requiresValue(for type: DatabaseType) -> Bool {
+        let mode = PluginManager.shared.connectionMode(for: type)
+        return mode == .fileBased || (mode == .apiOnly && showsBuiltInField(for: type))
+    }
+}

TablePro/Views/ConnectionForm/ViewModels/NetworkPaneViewModel.swift

@@ -78,25 +78,14 @@ final class NetworkPaneViewModel: ObservableObject {
             .connection.hidesBuiltInDatabase ?? false
     }
 
-    /// Whether the form renders the built-in Database field. A driver opts out through
-    /// `hidesBuiltInDatabase` when it names its container some other way, or has none.
-    /// This is deliberately not `requiresAuthentication`: whether a driver needs credentials
-    /// says nothing about whether it accepts a database name.
+    /// This is deliberately not `requiresAuthentication`: whether a driver needs credentials says
+    /// nothing about whether it accepts a database name.
     var showsBuiltInDatabaseField: Bool {
-        switch connectionMode {
-        case .fileBased:
-            return false
-        case .apiOnly:
-            return PluginManager.shared.supportsDatabaseSwitching(for: type) && !hidesBuiltInDatabase
-        default:
-            return !hidesBuiltInDatabase
-        }
+        ConnectionDatabaseRequirement.showsBuiltInField(for: type)
     }
 
-    /// Never require a value the form does not render. A file-based connection stores its
-    /// path in `database` and renders it as the Database File field.
     var requiresDatabaseValue: Bool {
-        connectionMode == .fileBased || (connectionMode == .apiOnly && showsBuiltInDatabaseField)
+        ConnectionDatabaseRequirement.requiresValue(for: type)
     }
 
     var validationIssues: [String] {

TablePro/Views/Main/Extensions/MainContentCoordinator+TabClosing.swift

@@ -38,6 +38,16 @@ extension MainContentCoordinator {
     /// Closed Tab, which could only reopen onto an error, and the saved tab set is never cleared,
     /// because an emptied tab list is not consent to forget it.
     func closeTabsForRemovedObjects(ids: [UUID]) {
+        /// Discarded rather than flushed, which is what the user-close path does. The object is
+        /// gone and the adoption has already cleared its saved settings, so a write from the
+        /// teardown would put the layout back after the clear.
+        ///
+        /// Only when the closed tabs include the selected one, because that is the only tab with a
+        /// mounted grid: a pending width belongs to it, and dropping an unrelated table would
+        /// otherwise throw away a resize the reader had just made somewhere else.
+        if let selectedId = tabManager.selectedTabId, ids.contains(selectedId) {
+            dataTabDelegate?.tableViewCoordinator?.discardPendingColumnLayoutPersistence()
+        }
         for id in ids {
             guard let tab = tabManager.tabs.first(where: { $0.id == id }) else { continue }
             releaseResources(of: tab)

TablePro/Views/Results/Extensions/DataGridView+Selection.swift

@@ -43,6 +43,17 @@ extension TableViewCoordinator {
         }
     }
 
+    /// Throws the pending layout away instead of writing it, for a table that is gone.
+    ///
+    /// A drop clears the table's saved layout and then closes its tab, and the teardown runs on a
+    /// later run-loop turn, so a flush there would write the layout back over the clear and mark it
+    /// dirty for sync again, waiting for a table recreated with the same name.
+    func discardPendingColumnLayoutPersistence() {
+        layoutPersistTask?.cancel()
+        layoutPersistTask = nil
+        pendingColumnLayoutPersistence = nil
+    }
+
     func flushPendingColumnLayoutPersistence() {
         layoutPersistTask?.cancel()
         layoutPersistTask = nil