diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d642d4ee..830f5a9a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Saving a table structure change with more than one connection open no longer applies the change to a different connection or jumps the view back to it. The save now runs against the connection, database, and schema the edited table belongs to, and stops with an error instead of writing if it cannot reach them. (#2015) - Saving on a MySQL or MariaDB server that starts sessions read-only no longer fails with "Cannot execute statement in a READ ONLY transaction". TablePro marks a transaction read-write before it writes instead of inheriting the server default. Same for PostgreSQL, CockroachDB, and Redshift. (#2009) - Changing Safe Mode in the connection form now applies to an open connection instead of waiting for a reconnect. (#2009) - A read-only error now says whether the database server or Safe Mode refused the write. (#2009) diff --git a/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift b/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift index 55b7cabb1..b015b766e 100644 --- a/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift +++ b/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift @@ -34,8 +34,8 @@ extension DatabaseManager { } } removeSessionEntry(for: connectionId) - if currentSessionId == connectionId { - currentSessionId = nil + if lastActiveSessionId == connectionId { + lastActiveSessionId = nil } } } diff --git a/TablePro/Core/Database/DatabaseManager+Health.swift b/TablePro/Core/Database/DatabaseManager+Health.swift index 891266258..835b28b12 100644 --- a/TablePro/Core/Database/DatabaseManager+Health.swift +++ b/TablePro/Core/Database/DatabaseManager+Health.swift @@ -227,12 +227,6 @@ extension DatabaseManager { } } - /// Reconnect the current session (called from toolbar Reconnect button) - func reconnectCurrentSession() async { - guard let sessionId = currentSessionId else { return } - await reconnectSession(sessionId) - } - /// Reconnect a specific session by ID func reconnectSession(_ sessionId: UUID) async { guard let session = activeSessions[sessionId] else { return } diff --git a/TablePro/Core/Database/DatabaseManager+Queries.swift b/TablePro/Core/Database/DatabaseManager+Queries.swift index fe4689d58..64f68a870 100644 --- a/TablePro/Core/Database/DatabaseManager+Queries.swift +++ b/TablePro/Core/Database/DatabaseManager+Queries.swift @@ -33,41 +33,6 @@ extension DatabaseManager { return try await operation() } - /// Execute a query on the current session - func execute(query: String) async throws -> QueryResult { - guard let sessionId = currentSessionId, let driver = activeDriver else { - throw DatabaseError.notConnected - } - - let result = try await trackOperation(sessionId: sessionId) { - try await driver.execute(query: query) - } - MacAnalyticsProvider.shared.markFirstQueryExecuted() - return result - } - - /// Fetch tables from the current session - func fetchTables() async throws -> [TableInfo] { - guard let sessionId = currentSessionId, let driver = activeDriver else { - throw DatabaseError.notConnected - } - - return try await trackOperation(sessionId: sessionId) { - try await driver.fetchTables() - } - } - - /// Fetch columns for a table from the current session - func fetchColumns(table: String) async throws -> [ColumnInfo] { - guard let sessionId = currentSessionId, let driver = activeDriver else { - throw DatabaseError.notConnected - } - - return try await trackOperation(sessionId: sessionId) { - try await driver.fetchColumns(table: table) - } - } - /// Test a connection without keeping it open func testConnection( _ connection: DatabaseConnection, diff --git a/TablePro/Core/Database/DatabaseManager+Schema.swift b/TablePro/Core/Database/DatabaseManager+Schema.swift index 1853bd28f..cd8edb864 100644 --- a/TablePro/Core/Database/DatabaseManager+Schema.swift +++ b/TablePro/Core/Database/DatabaseManager+Schema.swift @@ -13,28 +13,15 @@ import TableProPluginKit // MARK: - Schema Changes extension DatabaseManager { - /// Execute schema changes (ALTER TABLE, CREATE INDEX, etc.) in a transaction - func executeSchemaChanges( - tableName: String, - changes: [SchemaChange], - databaseType: DatabaseType - ) async throws { - guard let sessionId = currentSessionId else { - throw DatabaseError.notConnected - } - try await executeSchemaChanges( - tableName: tableName, - changes: changes, - databaseType: databaseType, - connectionId: sessionId - ) - } - - /// Execute schema changes using an explicit connection ID (session-scoped) + /// Execute schema changes (ALTER TABLE, CREATE INDEX, etc.) in a transaction. + /// The connection, database, and schema all come from the caller's own tab, never + /// from ambient session state that another window or tab can move. func executeSchemaChanges( tableName: String, changes: [SchemaChange], databaseType: DatabaseType, + databaseName: String, + schemaName: String?, connectionId: UUID ) async throws { guard let driver = driver(for: connectionId) else { @@ -42,6 +29,13 @@ extension DatabaseManager { } try await trackOperation(sessionId: connectionId) { + try await pinDatabaseBeforeSchemaChange( + databaseName: databaseName, + schemaName: schemaName, + databaseType: databaseType, + connectionId: connectionId + ) + // For PostgreSQL PK modification, query the actual constraint name let pkConstraintName = await fetchPrimaryKeyConstraintName( tableName: tableName, @@ -126,6 +120,54 @@ extension DatabaseManager { } } + /// Point the shared session driver at the database and schema the edited table + /// belongs to. Sibling tabs on the same connection move that driver, so a save must + /// re-pin before it writes. A failed pin aborts the save: a misdirected DDL is + /// silent and often irreversible, unlike a misdirected read. + /// + /// Switching database on a schema-grouped engine drops the driver back to the engine's + /// default schema, and those drivers qualify their DDL with whatever schema they are + /// currently on, so the schema has to be restored before any statement is generated. + private func pinDatabaseBeforeSchemaChange( + databaseName: String, + schemaName: String?, + databaseType: DatabaseType, + connectionId: UUID + ) async throws { + guard !databaseName.isEmpty, + !pluginManager.requiresReconnectForDatabaseSwitch(for: databaseType), + databaseName != activeSessions[connectionId]?.activeDatabase + else { + return + } + + let targetSchema = resolvedSchemaName(schemaName, for: connectionId) + + do { + try await switchDatabase(to: databaseName, for: connectionId, persist: false) + try await restoreSchemaAfterDatabasePin(targetSchema, for: connectionId) + } catch { + throw DatabaseError.queryFailed( + String( + format: String(localized: "Could not switch to database %@ before applying schema changes: %@"), + databaseName, + error.localizedDescription + ) + ) + } + } + + private func restoreSchemaAfterDatabasePin(_ targetSchema: String?, for connectionId: UUID) async throws { + guard let targetSchema, !targetSchema.isEmpty, + let schemaDriver = driver(for: connectionId) as? SchemaSwitchable, + schemaDriver.currentSchema != targetSchema + else { + return + } + + try await switchSchema(to: targetSchema, for: connectionId) + } + /// Query the actual primary key constraint name for PostgreSQL. /// Returns nil if the database is not PostgreSQL, no PK modification is pending, /// or the query fails (caller falls back to `{table}_pkey` convention). diff --git a/TablePro/Core/Database/DatabaseManager+Sessions.swift b/TablePro/Core/Database/DatabaseManager+Sessions.swift index 821cac485..3a2be2db8 100644 --- a/TablePro/Core/Database/DatabaseManager+Sessions.swift +++ b/TablePro/Core/Database/DatabaseManager+Sessions.swift @@ -42,7 +42,7 @@ extension DatabaseManager { session.status = .connecting setSession(session, for: connection.id) } - currentSessionId = connection.id + lastActiveSessionId = connection.id let effectiveConnection: DatabaseConnection do { @@ -196,8 +196,8 @@ extension DatabaseManager { internal func finalizeConnectionFailure(for connectionId: UUID, cancelled: Bool) { guard !cancelled else { return } removeSessionEntry(for: connectionId) - if currentSessionId == connectionId { - currentSessionId = activeSessions.keys.first + if lastActiveSessionId == connectionId { + lastActiveSessionId = activeSessions.keys.first } } @@ -330,7 +330,7 @@ extension DatabaseManager { func switchToSession(_ sessionId: UUID) { guard activeSessions[sessionId] != nil else { return } - currentSessionId = sessionId + lastActiveSessionId = sessionId updateSession(sessionId) { session in session.markActive() } @@ -382,11 +382,11 @@ extension DatabaseManager { SharedSidebarState.removeConnection(sessionId) SidebarViewModel.removeConnection(sessionId) - if currentSessionId == sessionId { + if lastActiveSessionId == sessionId { if let nextSessionId = activeSessions.keys.first { switchToSession(nextSessionId) } else { - currentSessionId = nil + lastActiveSessionId = nil } } lifecycleLogger.info( diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 2149a1334..420ee8c5c 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -41,8 +41,13 @@ final class DatabaseManager { /// counter to avoid cross-connection re-renders. internal(set) var connectionStatusVersions: [UUID: Int] = [:] - /// Currently selected session ID (displayed in UI) - internal var currentSessionId: UUID? + /// Best-effort "most recently activated" connection. Window focus never re-anchors it, + /// so it is only valid for UI highlighting (which connection the switcher marks active) + /// and as a fallback for entry points that have no window of their own, such as a new + /// contentless window or a file opened from Finder. Never resolve the target of an + /// operation through it: read the connection id from the window or tab that owns the + /// operation instead. + internal var lastActiveSessionId: UUID? /// Health monitors for active connections (MySQL/PostgreSQL only) @ObservationIgnored internal var healthMonitors: [UUID: ConnectionHealthMonitor] = [:] @@ -68,17 +73,12 @@ final class DatabaseManager { /// before touching shared session state and discards its driver when it lost. @ObservationIgnored internal var connectionAttempts = ConnectionAttemptRegistry() - /// Current session (computed from currentSessionId) - var currentSession: ConnectionSession? { - guard let sessionId = currentSessionId else { return nil } + /// Session for `lastActiveSessionId`, subject to the same caveats. + var lastActiveSession: ConnectionSession? { + guard let sessionId = lastActiveSessionId else { return nil } return activeSessions[sessionId] } - /// Current driver (for convenience) - var activeDriver: DatabaseDriver? { - currentSession?.driver - } - /// Resolve the driver for a specific connection (session-scoped, no global state) func driver(for connectionId: UUID) -> DatabaseDriver? { activeSessions[connectionId]?.driver @@ -109,11 +109,6 @@ final class DatabaseManager { return sessionSchema } - /// Current connection status - var status: ConnectionStatus { - currentSession?.status ?? .disconnected - } - internal init( connectionStorage: ConnectionStorage = .shared, appSettingsStorage: AppSettingsStorage = .shared, diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index 3ab3eee92..44ab73a42 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -89,7 +89,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi var resolvedSession: ConnectionSession? if let connectionId = payload?.connectionId { resolvedSession = DatabaseManager.shared.activeSessions[connectionId] - } else if let currentId = DatabaseManager.shared.currentSessionId { + } else if let currentId = DatabaseManager.shared.lastActiveSessionId { resolvedSession = DatabaseManager.shared.activeSessions[currentId] } self.currentSession = resolvedSession @@ -243,7 +243,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi guard closingSessionId == nil else { return } let sessions = DatabaseManager.shared.activeSessions - let connectionId = payload?.connectionId ?? currentSession?.id ?? DatabaseManager.shared.currentSessionId + let connectionId = payload?.connectionId ?? currentSession?.id ?? DatabaseManager.shared.lastActiveSessionId guard let sid = connectionId else { if currentSession != nil { currentSession = nil } diff --git a/TablePro/Core/Services/Infrastructure/TabRouter.swift b/TablePro/Core/Services/Infrastructure/TabRouter.swift index 55c459094..6e9ad26a6 100644 --- a/TablePro/Core/Services/Infrastructure/TabRouter.swift +++ b/TablePro/Core/Services/Infrastructure/TabRouter.swift @@ -352,7 +352,7 @@ internal final class TabRouter { return } - if let session = DatabaseManager.shared.currentSession { + if let session = DatabaseManager.shared.lastActiveSession { let content = await Task.detached(priority: .userInitiated) { () -> String? in try? String(contentsOf: url, encoding: .utf8) }.value diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index ee5abbfff..9f318f723 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -522,6 +522,12 @@ struct MainEditorContentView: View { } } + private func structureDatabaseName(for tab: QueryTab) -> String { + tab.tableContext.databaseName.isEmpty + ? coordinator.activeDatabaseName + : tab.tableContext.databaseName + } + @ViewBuilder private func resultsSection(tab: QueryTab) -> some View { VStack(spacing: 0) { @@ -532,11 +538,13 @@ struct MainEditorContentView: View { TableStructureView( tableName: tableName, connection: connection, + databaseName: structureDatabaseName(for: tab), + schemaName: tab.tableContext.schemaName, toolbarState: coordinator.toolbarState, coordinator: coordinator, selectionState: selectionState ) - .id(tableName) + .id("\(tab.tableContext.databaseName).\(tableName)") .frame(maxHeight: .infinity) } case .json: diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+ChangeGuard.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+ChangeGuard.swift index ef8fbb29b..cb4d8d3a8 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+ChangeGuard.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+ChangeGuard.swift @@ -27,8 +27,7 @@ extension MainContentCoordinator { } Task { - let window = NSApp.keyWindow - let confirmed = await confirmDiscardChanges(action: action, window: window) + let confirmed = await confirmDiscardChanges(action: action, window: contentWindow) if confirmed { changeManager.clearChangesAndUndoHistory() } diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift index 30defffb3..857d9f03e 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift @@ -62,7 +62,7 @@ extension MainContentCoordinator { } Task { - let confirmed = await confirmDiscardChanges(action: .refresh, window: NSApp.keyWindow) + let confirmed = await confirmDiscardChanges(action: .refresh, window: contentWindow) guard confirmed else { return } onDiscard() changeManager.clearChangesAndUndoHistory() diff --git a/TablePro/Views/Structure/TableStructureView+Schema.swift b/TablePro/Views/Structure/TableStructureView+Schema.swift index e80efb18b..33da4428a 100644 --- a/TablePro/Views/Structure/TableStructureView+Schema.swift +++ b/TablePro/Views/Structure/TableStructureView+Schema.swift @@ -93,7 +93,10 @@ extension TableStructureView { try await DatabaseManager.shared.executeSchemaChanges( tableName: tableName, changes: changes, - databaseType: connection.type + databaseType: connection.type, + databaseName: databaseName, + schemaName: schemaName, + connectionId: connection.id ) tabData.markAllStale() diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index cbb4718a6..e808ea988 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -19,6 +19,8 @@ struct TableStructureView: View { static let structurePasteboardType = NSPasteboard.PasteboardType("com.TablePro.structure") let tableName: String let connection: DatabaseConnection + let databaseName: String + let schemaName: String? let toolbarState: ConnectionToolbarState let coordinator: MainContentCoordinator? let selectionState: GridSelectionState @@ -59,12 +61,16 @@ struct TableStructureView: View { init( tableName: String, connection: DatabaseConnection, + databaseName: String, + schemaName: String?, toolbarState: ConnectionToolbarState, coordinator: MainContentCoordinator?, selectionState: GridSelectionState ) { self.tableName = tableName self.connection = connection + self.databaseName = databaseName + self.schemaName = schemaName self.toolbarState = toolbarState self.coordinator = coordinator self.selectionState = selectionState @@ -477,6 +483,8 @@ struct TableStructureView: View { username: "root", type: .mysql ), + databaseName: "test", + schemaName: nil, toolbarState: ConnectionToolbarState(), coordinator: nil, selectionState: GridSelectionState() diff --git a/TablePro/Views/Toolbar/ConnectionSwitcherPopover.swift b/TablePro/Views/Toolbar/ConnectionSwitcherPopover.swift index 41ff1fa3b..ac4595bd0 100644 --- a/TablePro/Views/Toolbar/ConnectionSwitcherPopover.swift +++ b/TablePro/Views/Toolbar/ConnectionSwitcherPopover.swift @@ -41,7 +41,7 @@ struct ConnectionSwitcherPopover: View { } private var currentSessionId: UUID? { - DatabaseManager.shared.currentSessionId + DatabaseManager.shared.lastActiveSessionId } private var sortedSessions: [ConnectionSession] { diff --git a/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift b/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift index e95693199..a2726911e 100644 --- a/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift +++ b/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift @@ -42,40 +42,40 @@ struct CancelledConnectionCleanupTests { #expect(DatabaseManager.shared.activeSessions[id] == nil) } - @Test("Cancelled finalize keeps currentSessionId untouched") - func cancelledKeepsCurrentSessionId() { + @Test("Cancelled finalize keeps lastActiveSessionId untouched") + func cancelledKeepsLastActiveSessionId() { let id = UUID() DatabaseManager.shared.injectSession( ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Retry")), for: id ) - DatabaseManager.shared.currentSessionId = id + DatabaseManager.shared.lastActiveSessionId = id defer { DatabaseManager.shared.removeSession(for: id) - DatabaseManager.shared.currentSessionId = nil + DatabaseManager.shared.lastActiveSessionId = nil } DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: true) - #expect(DatabaseManager.shared.currentSessionId == id) + #expect(DatabaseManager.shared.lastActiveSessionId == id) } - @Test("Genuine failure clears currentSessionId when no other session remains") - func genuineFailureClearsCurrentSessionId() { + @Test("Genuine failure clears lastActiveSessionId when no other session remains") + func genuineFailureClearsLastActiveSessionId() { let id = UUID() DatabaseManager.shared.injectSession( ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Failed")), for: id ) - DatabaseManager.shared.currentSessionId = id + DatabaseManager.shared.lastActiveSessionId = id defer { DatabaseManager.shared.removeSession(for: id) - DatabaseManager.shared.currentSessionId = nil + DatabaseManager.shared.lastActiveSessionId = nil } DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: false) - #expect(DatabaseManager.shared.currentSessionId != id) + #expect(DatabaseManager.shared.lastActiveSessionId != id) } @Test("Cancelling a pending connection invalidates the attempt still in flight") @@ -94,7 +94,7 @@ struct CancelledConnectionCleanupTests { #expect(DatabaseManager.shared.activeSessions[id] == nil) } - @Test("Genuine failure moves currentSessionId to a remaining session") + @Test("Genuine failure moves lastActiveSessionId to a remaining session") func genuineFailureSwitchesToRemainingSession() { let failedId = UUID() let otherId = UUID() @@ -106,16 +106,19 @@ struct CancelledConnectionCleanupTests { ConnectionSession(connection: TestFixtures.makeConnection(id: otherId, name: "Other")), for: otherId ) - DatabaseManager.shared.currentSessionId = failedId + DatabaseManager.shared.lastActiveSessionId = failedId defer { DatabaseManager.shared.removeSession(for: failedId) DatabaseManager.shared.removeSession(for: otherId) - DatabaseManager.shared.currentSessionId = nil + DatabaseManager.shared.lastActiveSessionId = nil } DatabaseManager.shared.finalizeConnectionFailure(for: failedId, cancelled: false) - #expect(DatabaseManager.shared.currentSessionId == otherId) + let moved = DatabaseManager.shared.lastActiveSessionId + #expect(moved != failedId) + #expect(moved != nil) + #expect(moved.flatMap { DatabaseManager.shared.activeSessions[$0] } != nil) #expect(DatabaseManager.shared.activeSessions[otherId] != nil) } } diff --git a/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift b/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift new file mode 100644 index 000000000..a638d72c8 --- /dev/null +++ b/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift @@ -0,0 +1,232 @@ +// +// DatabaseManagerSchemaChangeRoutingTests.swift +// TableProTests +// +// Pins the fix for #2015: a table structure save must run its DDL on the connection +// and database the editing tab owns, never on whichever session was activated last. +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private class SchemaRoutingBaseDriver { + var supportsSchemas: Bool { false } + var supportsTransactions: Bool { false } + var currentSchema: String? { nil } + var serverVersion: String? { nil } + + func connect() async throws {} + func disconnect() {} + + func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + + func fetchDatabases() async throws -> [String] { [] } + func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} + +private final class SchemaRoutingDriver: SchemaRoutingBaseDriver, PluginDatabaseDriver { + private(set) var executedQueries: [String] = [] + private(set) var switchedDatabases: [String] = [] + var switchDatabaseError: Error? + private var schema: String? + + override var supportsSchemas: Bool { true } + override var currentSchema: String? { schema } + + init(currentSchema: String? = nil) { + self.schema = currentSchema + super.init() + } + + func execute(query: String) async throws -> PluginQueryResult { + executedQueries.append(query) + return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func switchDatabase(to database: String) async throws { + if let switchDatabaseError { + throw switchDatabaseError + } + switchedDatabases.append(database) + } + + func switchSchema(to schema: String) async throws { + self.schema = schema + } + + func generateAddColumnSQL(table: String, column: PluginColumnDefinition) -> String? { + "ALTER TABLE \(qualified(table)) ADD COLUMN `\(column.name)` \(column.dataType)" + } + + private func qualified(_ table: String) -> String { + guard let schema, !schema.isEmpty else { return "`\(table)`" } + return "`\(schema)`.`\(table)`" + } +} + +@Suite("DatabaseManager schema change routing", .serialized) +@MainActor +struct DatabaseManagerSchemaChangeRoutingTests { + private static func makeAddColumnChange(named name: String = "notes") -> SchemaChange { + var column = EditableColumnDefinition.placeholder() + column.name = name + column.dataType = "TEXT" + return .addColumn(column) + } + + private static func makeSession( + type: DatabaseType = .mysql, + database: String = "testdb", + currentDatabase: String? = nil, + currentSchema: String? = nil + ) -> (DatabaseConnection, SchemaRoutingDriver) { + let connection = TestFixtures.makeConnection(database: database, type: type) + let pluginDriver = SchemaRoutingDriver(currentSchema: currentSchema) + let adapter = PluginDriverAdapter(connection: connection, pluginDriver: pluginDriver) + var session = ConnectionSession(connection: connection, driver: adapter) + session.currentDatabase = currentDatabase ?? database + session.currentSchema = currentSchema + DatabaseManager.shared.injectSession(session, for: connection.id) + return (connection, pluginDriver) + } + + @Test("Schema changes run on the requested connection, not the last activated one") + func schemaChangeUsesRequestedConnection() async throws { + let (connectionA, driverA) = Self.makeSession(database: "alpha") + let (connectionB, driverB) = Self.makeSession(database: "beta") + DatabaseManager.shared.lastActiveSessionId = connectionA.id + defer { + DatabaseManager.shared.removeSession(for: connectionA.id) + DatabaseManager.shared.removeSession(for: connectionB.id) + DatabaseManager.shared.lastActiveSessionId = nil + } + + try await DatabaseManager.shared.executeSchemaChanges( + tableName: "orders", + changes: [Self.makeAddColumnChange()], + databaseType: .mysql, + databaseName: "beta", + schemaName: nil, + connectionId: connectionB.id + ) + + #expect(driverB.executedQueries.count == 1) + #expect(driverB.executedQueries.first?.contains("ADD COLUMN") == true) + #expect(driverA.executedQueries.isEmpty) + } + + @Test("Schema changes pin the editing tab's database before running any DDL") + func schemaChangePinsDatabaseFirst() async throws { + let (connection, driver) = Self.makeSession(database: "orders", currentDatabase: "inventory") + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + try await DatabaseManager.shared.executeSchemaChanges( + tableName: "orders", + changes: [Self.makeAddColumnChange()], + databaseType: .mysql, + databaseName: "orders", + schemaName: nil, + connectionId: connection.id + ) + + #expect(driver.switchedDatabases == ["orders"]) + #expect(DatabaseManager.shared.session(for: connection.id)?.currentDatabase == "orders") + #expect(driver.executedQueries.count == 1) + } + + @Test("Schema changes skip the switch when the session is already on the tab's database") + func schemaChangeSkipsRedundantSwitch() async throws { + let (connection, driver) = Self.makeSession(database: "orders") + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + try await DatabaseManager.shared.executeSchemaChanges( + tableName: "orders", + changes: [Self.makeAddColumnChange()], + databaseType: .mysql, + databaseName: "orders", + schemaName: nil, + connectionId: connection.id + ) + + #expect(driver.switchedDatabases.isEmpty) + #expect(driver.executedQueries.count == 1) + } + + @Test("A failed database pin aborts the save before any DDL runs") + func failedDatabasePinAbortsSave() async throws { + let (connection, driver) = Self.makeSession(database: "orders", currentDatabase: "inventory") + driver.switchDatabaseError = DatabaseError.queryFailed("unknown database") + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + await #expect(throws: DatabaseError.self) { + try await DatabaseManager.shared.executeSchemaChanges( + tableName: "orders", + changes: [Self.makeAddColumnChange()], + databaseType: .mysql, + databaseName: "orders", + schemaName: nil, + connectionId: connection.id + ) + } + + #expect(driver.executedQueries.isEmpty) + } + + @Test("Engines that need a reconnect to switch database are never switched mid-save") + func reconnectRequiredEngineIsNotSwitched() async throws { + let (connection, driver) = Self.makeSession( + type: .postgresql, + database: "orders", + currentDatabase: "inventory" + ) + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + try await DatabaseManager.shared.executeSchemaChanges( + tableName: "orders", + changes: [Self.makeAddColumnChange()], + databaseType: .postgresql, + databaseName: "orders", + schemaName: nil, + connectionId: connection.id + ) + + #expect(driver.switchedDatabases.isEmpty) + #expect(driver.executedQueries.count == 1) + } + + @Test("A schema-grouped engine keeps the edited table's schema across the database pin") + func schemaGroupedEngineKeepsTableSchema() async throws { + let (connection, driver) = Self.makeSession( + type: .mssql, + database: "orders", + currentDatabase: "inventory", + currentSchema: "sales" + ) + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + try await DatabaseManager.shared.executeSchemaChanges( + tableName: "orders", + changes: [Self.makeAddColumnChange()], + databaseType: .mssql, + databaseName: "orders", + schemaName: "sales", + connectionId: connection.id + ) + + #expect(driver.switchedDatabases == ["orders"]) + #expect(driver.currentSchema == "sales") + #expect(driver.executedQueries.first?.contains("`sales`.`orders`") == true) + } +}