diff --git a/CHANGELOG.md b/CHANGELOG.md index 36e28b1357..70557841b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -363,6 +363,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Table opened in one database shown in Open Quickly's Recent in every other database, and opened there. - Open Quickly's Recent split between the Connections scope and the other scopes, each showing about half. - MySQL and MariaDB column defaults on iPhone and iPad missing for DEFAULT NULL, and string defaults shown unquoted. +- Structure and Create Table SQL Preview disagreeing with Save on the schema, primary key name or a SQLite foreign key. +- Row import creating its new table in another schema than its rows, and PGlite primary key changes failing to save. ### Security diff --git a/TablePro/Core/Database/DatabaseManager+Schema.swift b/TablePro/Core/Database/DatabaseManager+Schema.swift index 11a3846b6d..9c50ad1177 100644 --- a/TablePro/Core/Database/DatabaseManager+Schema.swift +++ b/TablePro/Core/Database/DatabaseManager+Schema.swift @@ -13,42 +13,21 @@ import TableProPluginKit // MARK: - Schema Changes extension DatabaseManager { - /// Execute schema changes (ALTER TABLE, CREATE INDEX, etc.) in a transaction of their own, + /// Execute schema statements (ALTER TABLE, CREATE INDEX, etc.) in a transaction of their own, /// on the schema change route rather than the session driver a query tab may have left /// mid-transaction. The connection, database and schema all come from the editing tab's /// own scope, never from ambient session state that another window or tab can move. /// - /// Authorization sits between two scoped blocks rather than inside one: it awaits a - /// confirmation sheet and Touch ID, and holding the connection's driver gate across a - /// human prompt would freeze every other tab on that connection. + /// Authorization sits outside the scoped block: it awaits a confirmation sheet and Touch ID, + /// and holding the connection's driver gate across a human prompt would freeze every other + /// tab on that connection. func executeSchemaChanges( - tableName: String, - changes: [SchemaChange], + _ statements: [SchemaStatement], databaseType: DatabaseType, scope: DatabaseScope ) async throws { let route = schemaChangeRoute(for: scope) - let statements = try await withScopedDriver( - scope: scope, route: route, cancellation: .untracked - ) { driver in - let pkConstraintName = await Self.fetchPrimaryKeyConstraintName( - tableName: tableName, - databaseType: databaseType, - changes: changes, - driver: driver - ) - guard let resolvedPluginDriver = (driver as? PluginDriverAdapter)?.schemaPluginDriver else { - throw DatabaseError.unsupportedOperation - } - let generator = SchemaStatementGenerator( - tableName: tableName, - primaryKeyConstraintName: pkConstraintName, - pluginDriver: resolvedPluginDriver - ) - return try generator.generate(changes: changes) - } - let combinedSQL = statements.map(\.sql).joined(separator: "\n") let schemaKind: OperationKind = QueryClassifier.classifyTier(combinedSQL, databaseType: databaseType) == .destructive @@ -229,59 +208,4 @@ extension DatabaseManager { .changed(CatalogChange(connectionId: scope.connectionId, database: scope.database, kinds: .tables)) ) } - - /// 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). - private static func fetchPrimaryKeyConstraintName( - tableName: String, - databaseType: DatabaseType, - changes: [SchemaChange], - driver: DatabaseDriver - ) async -> String? { - // Only needed for PostgreSQL PK modifications - guard databaseType == .postgresql || databaseType == .redshift - || databaseType == .cockroachdb || databaseType == .duckdb else { return nil } - guard - changes.contains(where: { - if case .modifyPrimaryKey = $0 { return true } - return false - }) - else { - return nil - } - - let escapedTable = tableName.replacingOccurrences(of: "'", with: "''") - let schema: String - if let schemaDriver = driver as? SchemaSwitchable, - let escaped = schemaDriver.escapedSchema { - schema = escaped - } else { - schema = "public" - } - let query = """ - SELECT con.conname - FROM pg_constraint con - JOIN pg_class rel ON rel.oid = con.conrelid - JOIN pg_namespace nsp ON nsp.oid = rel.relnamespace - WHERE rel.relname = '\(escapedTable)' - AND nsp.nspname = '\(schema)' - AND con.contype = 'p' - LIMIT 1 - """ - - do { - let result = try await driver.execute(query: query) - if let row = result.rows.first, let name = row[0].asText, !name.isEmpty { - return name - } - } catch { - // Query failed - fall back to convention in SchemaStatementGenerator - Self.logger.warning( - "Failed to query PK constraint name for '\(tableName)': \(error.localizedDescription)" - ) - } - - return nil - } } diff --git a/TablePro/Core/Database/DatabaseManager+SchemaComposition.swift b/TablePro/Core/Database/DatabaseManager+SchemaComposition.swift new file mode 100644 index 0000000000..3d21a28109 --- /dev/null +++ b/TablePro/Core/Database/DatabaseManager+SchemaComposition.swift @@ -0,0 +1,68 @@ +// +// DatabaseManager+SchemaComposition.swift +// TablePro +// + +import Foundation +import TableProPluginKit +import TableProSQLGrammar + +extension DatabaseManager { + func withSchemaComposer( + scope: DatabaseScope, + route: ScopedDriverRoute, + _ body: @Sendable @escaping (DatabaseDriver, any PluginDatabaseDriver) async throws -> T + ) async throws -> T { + try await withScopedDriver(scope: scope, route: route, cancellation: .untracked) { driver in + guard let pluginDriver = (driver as? PluginDriverAdapter)?.schemaPluginDriver else { + throw DatabaseError.unsupportedOperation + } + return try await body(driver, pluginDriver) + } + } + + func schemaChangeStatements( + tableName: String, + changes: [SchemaChange], + scope: DatabaseScope + ) async throws -> [SchemaStatement] { + try await withSchemaComposer(scope: scope, route: schemaChangeRoute(for: scope)) { driver, pluginDriver in + let constraintName = await PrimaryKeyConstraintLookup.constraintName( + tableName: tableName, + changes: changes, + driver: driver + ) + return try SchemaStatementGenerator( + tableName: tableName, + primaryKeyConstraintName: constraintName, + pluginDriver: pluginDriver + ).generate(changes: changes) + } + } + + func createTableStatements( + plan: CreateTablePlan, + scope: DatabaseScope + ) async throws -> CreateTableStatements { + guard plan.definition != nil else { + return CreateTableStatements(statements: [], issues: plan.issues, tableName: nil) + } + return try await withSchemaComposer(scope: scope, route: schemaChangeRoute(for: scope)) { _, pluginDriver in + await MainActor.run { + CreateTableStatementComposer.compose(plan: plan, driver: pluginDriver) + } + } + } + + func createTableStatements( + definition: PluginCreateTableDefinition, + scope: DatabaseScope, + route: ScopedDriverRoute + ) async throws -> [String] { + try await withSchemaComposer(scope: scope, route: route) { _, pluginDriver in + (pluginDriver.generateCreateTableStatements(definition: definition) ?? []) + .map { StatementBlank.trimming($0) } + .filter { !$0.isEmpty } + } + } +} diff --git a/TablePro/Core/SchemaTracking/PrimaryKeyConstraintLookup.swift b/TablePro/Core/SchemaTracking/PrimaryKeyConstraintLookup.swift new file mode 100644 index 0000000000..3aa0b98509 --- /dev/null +++ b/TablePro/Core/SchemaTracking/PrimaryKeyConstraintLookup.swift @@ -0,0 +1,45 @@ +// +// PrimaryKeyConstraintLookup.swift +// TablePro +// + +import Foundation +import os +import TableProPluginKit + +enum PrimaryKeyConstraintLookup { + private static let logger = Logger(subsystem: "com.TablePro", category: "PrimaryKeyConstraintLookup") + + static func constraintName( + tableName: String, + changes: [SchemaChange], + driver: DatabaseDriver + ) async -> String? { + guard changes.contains(where: dropsExistingPrimaryKey) else { return nil } + guard let schema = (driver as? SchemaSwitchable)?.escapedSchema else { return nil } + + let query = """ + SELECT CONSTRAINT_NAME + FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS + WHERE TABLE_SCHEMA = '\(schema)' + AND TABLE_NAME = '\(driver.escapeStringLiteral(tableName))' + AND CONSTRAINT_TYPE = 'PRIMARY KEY' + """ + + do { + let result = try await driver.execute(query: query) + guard let row = result.rows.first, let name = row.first?.asText, !name.isEmpty else { return nil } + return name + } catch { + logger.warning( + "Primary key constraint name lookup failed: \(error.publicLogShape, privacy: .public)" + ) + return nil + } + } + + private static func dropsExistingPrimaryKey(_ change: SchemaChange) -> Bool { + guard case .modifyPrimaryKey(let old, _) = change else { return false } + return !old.isEmpty + } +} diff --git a/TablePro/Core/Services/Export/ImportService.swift b/TablePro/Core/Services/Export/ImportService.swift index 4328a99d69..f79877b44f 100644 --- a/TablePro/Core/Services/Export/ImportService.swift +++ b/TablePro/Core/Services/Export/ImportService.swift @@ -52,6 +52,7 @@ final class ImportService: ObservableObject { from url: URL, formatId: String, encoding: String.Encoding, + scope: DatabaseScope, decompressedURL: URL? = nil, ownsDecompressedFile: Bool = false, knownStatementCount: Int? = nil, @@ -62,12 +63,6 @@ final class ImportService: ObservableObject { throw PluginImportError.importFailed("Import format '\(formatId)' not found") } - /// The scope the driver is already on, not the connection's saved default: a tab may have - /// moved it, and on an engine that reconnects to change database, pinning somewhere else - /// would refuse the import outright. - guard let scope = DatabaseManager.shared.browseScope(for: connection.id) else { - throw DatabaseError.notConnected - } let route = DatabaseManager.shared.executionRoute(for: scope) state = ImportState(isImporting: true) diff --git a/TablePro/Models/Schema/TableRebuildReviewRequest.swift b/TablePro/Models/Schema/TableRebuildReviewRequest.swift index 649ca9066f..4174cda86d 100644 --- a/TablePro/Models/Schema/TableRebuildReviewRequest.swift +++ b/TablePro/Models/Schema/TableRebuildReviewRequest.swift @@ -13,6 +13,13 @@ import TableProPluginKit /// first, and what the rebuild cannot carry over is named beside it. @MainActor struct TableRebuildReviewRequest: Identifiable { + struct Action { + /// What the confirming button says, which is the only thing that differs between a reorder + /// and a constraint change: both recreate the table, and the user is reading the same script. + let title: String + let perform: () async -> Void + } + let id = UUID() let tableName: String @@ -24,11 +31,7 @@ struct TableRebuildReviewRequest: Identifiable { let plan: PluginColumnReorderPlan - /// What the confirming button says, which is the only thing that differs between a reorder and - /// a constraint change: both recreate the table, and the user is reading the same script. - let actionTitle: String - - let perform: () async -> Void + let action: Action? var warning: String? { plan.caveats.isEmpty ? nil : plan.caveats.joined(separator: " ") @@ -36,5 +39,9 @@ struct TableRebuildReviewRequest: Identifiable { var isRunnable: Bool { plan.isRunnable } + var runnableAction: Action? { + isRunnable ? action : nil + } + var scriptStatements: [String] { plan.scriptStatements } } diff --git a/TablePro/Views/Import/ImportDialog.swift b/TablePro/Views/Import/ImportDialog.swift index 6d9acf1579..b04e9d510b 100644 --- a/TablePro/Views/Import/ImportDialog.swift +++ b/TablePro/Views/Import/ImportDialog.swift @@ -484,10 +484,17 @@ struct ImportDialog: View { importTask = Task { do { + /// The scope the driver is already on, not the connection's saved default: a tab may + /// have moved it, and on an engine that reconnects to change database, pinning + /// somewhere else would refuse the import outright. + guard let scope = DatabaseManager.shared.browseScope(for: connection.id) else { + throw DatabaseError.notConnected + } let result = try await service.importFile( from: url, formatId: selectedFormatId, encoding: selectedEncoding.encoding, + scope: scope, decompressedURL: decompressedURL, ownsDecompressedFile: ownsDecompressedFile, knownStatementCount: statementCount > 0 ? statementCount : nil diff --git a/TablePro/Views/Import/RowImportSheet.swift b/TablePro/Views/Import/RowImportSheet.swift index d2b455779d..785af9d252 100644 --- a/TablePro/Views/Import/RowImportSheet.swift +++ b/TablePro/Views/Import/RowImportSheet.swift @@ -13,7 +13,6 @@ import Combine import os import SwiftUI import TableProPluginKit -import TableProSQLGrammar struct RowImportSheet: View { @ObservedObject private var pluginManager = PluginManager.shared @@ -798,24 +797,33 @@ struct RowImportSheet: View { // MARK: - Import private func performImport() { + guard let scope = DatabaseManager.shared.browseScope(for: connection.id) else { + importError = DatabaseError.notConnected + showErrorDialog = true + return + } switch destination { case .existingTable: guard let table = selectedTargetTable else { return } - runImport(targetTable: table, mapping: existingMapping(), createTableStatements: nil) + runImport(targetTable: table, mapping: existingMapping(), newTable: nil, scope: scope) case .newTable: let name = newTableName.trimmingCharacters(in: .whitespaces) - guard !name.isEmpty, let statements = buildCreateTableStatements(tableName: name) else { - importError = NSError( - domain: "RowImport", code: -1, - userInfo: [NSLocalizedDescriptionKey: String(localized: "Could not build the CREATE TABLE statement")] - ) + guard !name.isEmpty, let definition = newTableDefinition(tableName: name) else { + importError = Self.createTableStatementError showErrorDialog = true return } - runImport(targetTable: name, mapping: newTableMapping(), createTableStatements: statements) + runImport(targetTable: name, mapping: newTableMapping(), newTable: definition, scope: scope) } } + private static var createTableStatementError: NSError { + NSError( + domain: "RowImport", code: -1, + userInfo: [NSLocalizedDescriptionKey: String(localized: "Could not build the CREATE TABLE statement")] + ) + } + private func existingMapping() -> [String: String] { var mapping: [String: String] = [:] for entry in mappings where entry.include { @@ -834,7 +842,7 @@ struct RowImportSheet: View { return mapping } - private func buildCreateTableStatements(tableName: String) -> [String]? { + private func newTableDefinition(tableName: String) -> PluginCreateTableDefinition? { let included = newColumns.filter { $0.include && !$0.name.trimmingCharacters(in: .whitespaces).isEmpty @@ -842,7 +850,7 @@ struct RowImportSheet: View { } guard !included.isEmpty else { return nil } - let definition = PluginCreateTableDefinition( + return PluginCreateTableDefinition( tableName: tableName, columns: included.map { column in PluginColumnDefinition( @@ -861,29 +869,28 @@ struct RowImportSheet: View { }, primaryKeyColumns: included.filter(\.isPrimaryKey).map(\.name) ) - - let pluginDriver = (DatabaseManager.shared.driver(for: connection.id) as? PluginDriverAdapter)?.schemaPluginDriver - let statements = pluginDriver?.generateCreateTableStatements(definition: definition)? - .map { StatementBlank.trimming($0) } - .filter { !$0.isEmpty } - guard let statements, !statements.isEmpty else { return nil } - return statements } - private func runImport(targetTable: String, mapping: [String: String], createTableStatements: [String]?) { + private func runImport( + targetTable: String, + mapping: [String: String], + newTable: PluginCreateTableDefinition?, + scope: DatabaseScope + ) { let service = ImportService(connection: connection) importService = service showProgressDialog = true importTask = Task { do { - if let createTableStatements { - try await prepareTable(named: targetTable, statements: createTableStatements) + if let newTable { + try await prepareTable(newTable, scope: scope) } let result = try await service.importFile( from: fileURL, formatId: formatId, encoding: .utf8, + scope: scope, targetTable: targetTable, columnMapping: mapping ) @@ -912,16 +919,23 @@ struct RowImportSheet: View { } @MainActor - private func prepareTable(named tableName: String, statements: [String]) async throws { + private func prepareTable(_ definition: PluginCreateTableDefinition, scope: DatabaseScope) async throws { + let tableName = definition.tableName + let statements = try await DatabaseManager.shared.createTableStatements( + definition: definition, + scope: scope, + route: DatabaseManager.shared.executionRoute(for: scope) + ) + guard !statements.isEmpty else { throw Self.createTableStatementError } let sql = statements.joined(separator: "\n") switch NewTableImportPlanner.plan( forTable: tableName, createTableSQL: sql, alreadyCreated: createdTables ) { case .create: - try await createTable(statements: statements) + try await createTable(statements: statements, scope: scope) createdTables[tableName] = sql case .reuseAfterClearing: - try await clearRows(of: tableName) + try await clearRows(of: tableName, scope: scope) case .nameTakenWithDifferentColumns: throw PluginImportError.importFailed( String( @@ -933,7 +947,7 @@ struct RowImportSheet: View { } @MainActor - private func clearRows(of tableName: String) async throws { + private func clearRows(of tableName: String, scope: DatabaseScope) async throws { let generator = try SQLStatementGenerator( tableName: tableName, columns: [], @@ -944,20 +958,22 @@ struct RowImportSheet: View { try await authorize( sql: sql, kind: .destructiveQuery, description: String(localized: "Clear Table") ) - try await runOnLeasedDriver(sql) + try await runOnLeasedDriver(sql, scope: scope) } /// One call per statement the driver wrote, because an engine that runs one statement per call refuses a table /// and its indexes sent together. - private func createTable(statements: [String]) async throws { + private func createTable(statements: [String], scope: DatabaseScope) async throws { let script = SQLScriptText(databaseType: connection.type).script(statements) try await authorize( sql: script, kind: .schemaMutation, description: String(localized: "Create Table") ) for statement in statements { - try await runOnLeasedDriver(statement) + try await runOnLeasedDriver(statement, scope: scope) } - CatalogChangeService.post(.changed(CatalogChange(connectionId: connection.id, kinds: .tables))) + CatalogChangeService.post( + .changed(CatalogChange(connectionId: connection.id, database: scope.database, kinds: .tables)) + ) } /// The sheet's own statements take the same lease the import does, one at a time and always @@ -965,10 +981,7 @@ struct RowImportSheet: View { /// across a safe-mode confirmation the user has not answered yet, and the gate is not /// reentrant, so the import that follows would then wait on a sheet waiting on the user. @MainActor - private func runOnLeasedDriver(_ sql: String) async throws { - guard let scope = DatabaseManager.shared.browseScope(for: connection.id) else { - throw DatabaseError.notConnected - } + private func runOnLeasedDriver(_ sql: String, scope: DatabaseScope) async throws { let route = DatabaseManager.shared.executionRoute(for: scope) _ = try await DatabaseManager.shared.withScopedDriver( scope: scope, diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 8e08645cd9..e05fcd3493 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -282,17 +282,17 @@ struct MainContentView: View { statements: request.scriptStatements, databaseType: connection.type, warning: request.warning, - primaryAction: request.isRunnable - ? SQLReviewSheet.PrimaryAction( - title: request.actionTitle, + primaryAction: request.runnableAction.map { action in + SQLReviewSheet.PrimaryAction( + title: action.title, isDestructive: true, perform: { - await request.perform() + await action.perform() coordinator.tableRebuildRequest = nil coordinator.activeSheet = nil } ) - : nil, + }, onOpenInEditor: { coordinator.openTableRebuildScriptInEditor(request) } diff --git a/TablePro/Views/Structure/CreateTableDraft.swift b/TablePro/Views/Structure/CreateTableDraft.swift index 57ab9d6104..8fc0b94f0d 100644 --- a/TablePro/Views/Structure/CreateTableDraft.swift +++ b/TablePro/Views/Structure/CreateTableDraft.swift @@ -6,6 +6,15 @@ import Combine import Foundation +internal struct CreateTableCompositionKey: Hashable { + let scope: DatabaseScope + let tableName: String + let options: CreateTableOptions + let columns: [EditableColumnDefinition] + let indexes: [EditableIndexDefinition] + let foreignKeys: [EditableForeignKeyDefinition] +} + /// A table definition in progress, held outside the view that edits it. /// /// A Create Table tab's whole content is unsaved by definition: nothing exists on the server until @@ -19,6 +28,18 @@ internal final class CreateTableDraft: ObservableObject { @Published internal var tableName = "" @Published internal var tableOptions = CreateTableOptions() + @Published internal private(set) var composed: CreateTableStatements? + @Published internal private(set) var compositionFailure: String? + + private var composedKey: CreateTableCompositionKey? + private var compositionGeneration = 0 + private var changeManagerForwarding: AnyCancellable? + + internal init() { + changeManagerForwarding = changeManager.objectWillChange + .sink { [weak self] in self?.objectWillChange.send() } + } + /// Whether the draft holds anything worth losing. A tab that has only just opened does not: the /// editor seeds one blank column so the grid has a row to show, which registers as a pending /// change without the user having typed anything. @@ -33,4 +54,54 @@ internal final class CreateTableDraft: ObservableObject { !$0.name.isEmpty || !$0.columns.isEmpty || !$0.referencedTable.isEmpty } } + + internal static func offersEngineOptions(for databaseType: DatabaseType) -> Bool { + databaseType == .mysql || databaseType == .mariadb + } + + internal func plan(for databaseType: DatabaseType) -> CreateTablePlan { + CreateTableDraftBuilder.plan( + tableName: tableName, + options: tableOptions, + columns: changeManager.workingColumns, + indexes: changeManager.workingIndexes, + foreignKeys: changeManager.workingForeignKeys, + dialect: ForeignKeyDialect.forType(databaseType), + includesEngineOptions: Self.offersEngineOptions(for: databaseType) + ) + } + + internal func compositionKey(scope: DatabaseScope) -> CreateTableCompositionKey { + CreateTableCompositionKey( + scope: scope, + tableName: tableName, + options: tableOptions, + columns: changeManager.workingColumns, + indexes: changeManager.workingIndexes, + foreignKeys: changeManager.workingForeignKeys + ) + } + + internal func recompose(databaseType: DatabaseType, scope: DatabaseScope) async { + let key = compositionKey(scope: scope) + guard key != composedKey else { return } + + compositionGeneration += 1 + let generation = compositionGeneration + do { + let statements = try await DatabaseManager.shared.createTableStatements( + plan: plan(for: databaseType), + scope: scope + ) + guard generation == compositionGeneration else { return } + composed = statements + composedKey = key + compositionFailure = nil + } catch is CancellationError { + return + } catch { + guard generation == compositionGeneration else { return } + compositionFailure = error.localizedDescription + } + } } diff --git a/TablePro/Views/Structure/CreateTableView.swift b/TablePro/Views/Structure/CreateTableView.swift index 145699d513..ca5f53f8de 100644 --- a/TablePro/Views/Structure/CreateTableView.swift +++ b/TablePro/Views/Structure/CreateTableView.swift @@ -136,6 +136,7 @@ struct CreateTableView: View { .onChange(of: selectedRows) { newRows in selectionState.indices = newRows } .onChange(of: selectedTab) { _ in updateGridDelegate() } .onChange(of: isReadyToCreate) { _ in updateCreateTablePendingState() } + .task(id: compositionKey) { await recomposeAfterPause() } .alert(String(localized: "Create Table Failed"), isPresented: $showError) { Button("OK") {} } message: { @@ -194,7 +195,7 @@ struct CreateTableView: View { } private var showMySQLOptions: Bool { - connection.type == .mysql || connection.type == .mariadb + CreateTableDraft.offersEngineOptions(for: connection.type) } // MARK: - Toolbar @@ -215,7 +216,7 @@ struct CreateTableView: View { /// The composed issues, not the plan's. A driver that cannot spell one of the statements, /// as Snowflake and Trino cannot spell `CREATE INDEX`, reports it only here, and reading the /// plan alone left Create Table enabled over a preview the app would then refuse to run. - let issues = currentStatements().issues + let issues = draft.composed?.issues ?? draft.plan(for: connection.type).issues return HStack(spacing: 8) { Button(action: { gridDelegate.dataGridAddRow() }) { @@ -369,70 +370,67 @@ struct CreateTableView: View { // MARK: - SQL Preview - /// Derived from the working rows rather than refreshed by an event. - /// - /// It used to be `@State` written by `onChange(of: reloadVersion)`, and `reloadVersion` is bumped - /// only by a schema load and a discard, never by an edit. What kept the preview honest was the - /// segment switch remounting this branch, so anything that changed the draft while the preview - /// was already on screen, undo among them, left a statement on screen that would not be run. + @ViewBuilder private var sqlPreviewView: some View { - let composed = currentStatements() - return Group { + if let composed = draft.composed { if composed.statements.isEmpty { - VStack(spacing: 8) { - Image(systemName: "doc.plaintext") - .font(.largeTitle) - .foregroundStyle(.secondary) - .accessibilityHidden(true) - Text(composed.issues.first?.qualifiedMessage - ?? String(localized: "Add columns to see the CREATE TABLE statement")) - .foregroundStyle(.secondary) - .multilineTextAlignment(.center) - } - .frame(maxWidth: .infinity, maxHeight: .infinity) + sqlPreviewPlaceholder(composed.issues.first?.qualifiedMessage) } else { DDLTextView(ddl: composed.preview, fontSize: .constant(13)) } + } else if let failure = draft.compositionFailure { + sqlPreviewPlaceholder(failure) + } else { + uncomposedPreview(plan: draft.plan(for: connection.type)) + } + } + + @ViewBuilder + private func uncomposedPreview(plan: CreateTablePlan) -> some View { + if plan.definition == nil { + sqlPreviewPlaceholder(plan.issues.first?.qualifiedMessage) + } else { + ProgressView() + .controlSize(.small) + .frame(maxWidth: .infinity, maxHeight: .infinity) } } + private func sqlPreviewPlaceholder(_ message: String?) -> some View { + VStack(spacing: 8) { + Image(systemName: "doc.plaintext") + .font(.largeTitle) + .foregroundStyle(.secondary) + .accessibilityHidden(true) + Text(message ?? String(localized: "Add columns to see the CREATE TABLE statement")) + .foregroundStyle(.secondary) + .multilineTextAlignment(.center) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + // Cell editing, row operations, undo/redo handled by CreateTableGridDelegate // MARK: - SQL Generation - /// Pure, so the toolbar can ask on every keystroke without touching the driver. - private var currentPlan: CreateTablePlan { - CreateTableDraftBuilder.plan( - tableName: draft.tableName, - options: draft.tableOptions, - columns: structureChangeManager.workingColumns, - indexes: structureChangeManager.workingIndexes, - foreignKeys: structureChangeManager.workingForeignKeys, - dialect: ForeignKeyDialect.forType(connection.type), - includesEngineOptions: showMySQLOptions - ) - } + private static let compositionPause: Duration = .milliseconds(150) - private func currentStatements() -> CreateTableStatements { - statements(composedWith: DatabaseManager.shared.driver(for: connection.id)) + private var compositionKey: CreateTableCompositionKey? { + scope.map { draft.compositionKey(scope: $0) } } - /// Several visual-editor drivers write their own current schema or catalog into the statement as - /// an explicit qualifier, so the driver the SQL is composed on decides where the table lands. - /// Composing on the session driver and executing on the tab's scope pinned only half of it. - private func statements(composedWith driver: DatabaseDriver?) -> CreateTableStatements { - let plan = currentPlan - guard let pluginDriver = (driver as? PluginDriverAdapter)?.schemaPluginDriver else { - return CreateTableStatements(statements: [], issues: plan.issues, tableName: nil) - } - return CreateTableStatementComposer.compose(plan: plan, driver: pluginDriver) + private func recomposeAfterPause() async { + guard let scope else { return } + try? await Task.sleep(for: Self.compositionPause) + guard !Task.isCancelled else { return } + await draft.recompose(databaseType: connection.type, scope: scope) } // MARK: - Create Table private var isReadyToCreate: Bool { - let composed = currentStatements() - return !isCreating && composed.issues.isEmpty && !composed.statements.isEmpty + guard !isCreating, let composed = draft.composed else { return false } + return composed.issues.isEmpty && !composed.statements.isEmpty } private func updateCreateTablePendingState() { @@ -448,8 +446,9 @@ struct CreateTableView: View { /// keep the app's own DDL off the user's connection. private func createTable() { guard !isCreating else { return } - guard currentStatements().issues.isEmpty else { - errorMessage = currentStatements().issues.map(\.qualifiedMessage).joined(separator: "\n") + let plan = draft.plan(for: connection.type) + guard plan.issues.isEmpty else { + errorMessage = plan.issues.map(\.qualifiedMessage).joined(separator: "\n") showError = true return } @@ -466,9 +465,7 @@ struct CreateTableView: View { Task { defer { isCreating = false } do { - let composed = try await DatabaseManager.shared.withMetadataDriver(scope: scope) { driver in - await MainActor.run { statements(composedWith: driver) } - } + let composed = try await DatabaseManager.shared.createTableStatements(plan: plan, scope: scope) guard composed.issues.isEmpty, !composed.statements.isEmpty else { errorMessage = composed.issues.map(\.qualifiedMessage).joined(separator: "\n") showError = true diff --git a/TablePro/Views/Structure/StructureEditingSession+Apply.swift b/TablePro/Views/Structure/StructureEditingSession+Apply.swift index 79d7365296..9a32c9d5b2 100644 --- a/TablePro/Views/Structure/StructureEditingSession+Apply.swift +++ b/TablePro/Views/Structure/StructureEditingSession+Apply.swift @@ -75,20 +75,41 @@ internal extension StructureEditingSession { return .refused } - /// An engine that cannot express this save as `ALTER` statements recreates the table - /// instead, and a rebuild is never run from a Save press. It is shown in full, with what it - /// cannot carry over, and confirmed before anything is dropped. - /// - /// Ahead of the destructive-changes prompt, not after it. The review sheet is already that - /// confirmation and shows the exact script rather than a list of descriptions, so asking - /// first would be two dialogs for one decision. The HIG's rule is one alert at a time. - if StructureTableRebuildHandler.requiresRebuild( - changes: changes, - support: PluginManager.shared.foreignKeyEditSupport(for: connection.type) - ) { - return await presentRebuildReview(changes: changes, coordinator: coordinator) + let planStart = ContinuousClock.Instant.now + let plan: StructureSavePlan + do { + plan = try await stagedSavePlan(for: changes) + } catch { + report(.failed(reason: error.localizedDescription), startedAt: planStart, coordinator: coordinator) + AlertHelper.showErrorSheet( + title: String(localized: "Error Applying Changes"), + message: error.localizedDescription, + window: coordinator?.contentWindow + ) + return .failed(error.localizedDescription) + } + + switch plan { + case .rebuild(let prepared): + /// An engine that cannot express this save as `ALTER` statements recreates the table + /// instead, and a rebuild is never run from a Save press. It is shown in full, with what + /// it cannot carry over, and confirmed before anything is dropped. + /// + /// Ahead of the destructive-changes prompt, not after it. The review sheet is already + /// that confirmation and shows the exact script rather than a list of descriptions, so + /// asking first would be two dialogs for one decision. The HIG's rule is one alert at a + /// time. + return presentRebuildReview(prepared, startedAt: planStart, coordinator: coordinator) + case .alter(let statements): + return await applyAlterStatements(statements, changes: changes, coordinator: coordinator) } + } + private func applyAlterStatements( + _ statements: [SchemaStatement], + changes: [SchemaChange], + coordinator: MainContentCoordinator? + ) async -> StructureSaveOutcome { let destructiveChanges = changes.filter(\.requiresDataMigration) if !destructiveChanges.isEmpty { let message = String( @@ -113,8 +134,7 @@ internal extension StructureEditingSession { do { try await DatabaseManager.shared.executeSchemaChanges( - tableName: tableName, - changes: changes, + statements, databaseType: connection.type, scope: scope ) @@ -138,49 +158,30 @@ internal extension StructureEditingSession { } } - /// Builds the rebuild script and hands it to the review sheet. + /// Hands the rebuild script to the review sheet. /// /// Returns `.refused` because at this point nothing has run and the edits are still staged, /// which is exactly what a close has to be stood down for. The apply happens in the sheet's own /// action if the user confirms it there. private func presentRebuildReview( - changes: [SchemaChange], + _ prepared: StructureRebuildPlanRunner.Prepared, + startedAt operationStart: ContinuousClock.Instant, coordinator: MainContentCoordinator? - ) async -> StructureSaveOutcome { + ) -> StructureSaveOutcome { guard let coordinator else { return .refused } - let operationStart = ContinuousClock.Instant.now - let reviewScope = scope - - do { - let prepared = try await StructureTableRebuildHandler.prepare( - changes: changes, - tableName: tableName, - scope: reviewScope - ) - coordinator.tableRebuildRequest = TableRebuildReviewRequest( - tableName: tableName, - scope: reviewScope, - plan: prepared.plan, - actionTitle: String(localized: "Apply and Rebuild"), + coordinator.tableRebuildRequest = TableRebuildReviewRequest( + tableName: prepared.tableName, + scope: prepared.scope, + plan: prepared.plan, + action: TableRebuildReviewRequest.Action( + title: String(localized: "Apply and Rebuild"), perform: { [weak coordinator] in - await self.runRebuild( - prepared, - startedAt: operationStart, - coordinator: coordinator - ) + await self.runRebuild(prepared, startedAt: operationStart, coordinator: coordinator) } ) - coordinator.activeSheet = .tableRebuildReview - return .refused - } catch { - report(.failed(reason: error.localizedDescription), startedAt: operationStart, coordinator: coordinator) - AlertHelper.showErrorSheet( - title: String(localized: "Error Applying Changes"), - message: error.localizedDescription, - window: coordinator.contentWindow - ) - return .failed(error.localizedDescription) - } + ) + coordinator.activeSheet = .tableRebuildReview + return .refused } /// Runs a confirmed rebuild and does everything a save owes the rest of the app afterwards. diff --git a/TablePro/Views/Structure/StructureSavePlan.swift b/TablePro/Views/Structure/StructureSavePlan.swift new file mode 100644 index 0000000000..004f829264 --- /dev/null +++ b/TablePro/Views/Structure/StructureSavePlan.swift @@ -0,0 +1,79 @@ +// +// StructureSavePlan.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +internal enum StructureSavePlan { + case alter([SchemaStatement]) + case rebuild(StructureRebuildPlanRunner.Prepared) + + internal var displayStatements: [String] { + switch self { + case .alter(let statements): + statements.map(\.sql) + case .rebuild(let prepared): + prepared.plan.scriptStatements + } + } +} + +internal extension StructureEditingSession { + func stagedSavePlan(for changes: [SchemaChange]) async throws -> StructureSavePlan { + let support = PluginManager.shared.foreignKeyEditSupport(for: connection.type) + guard StructureTableRebuildHandler.requiresRebuild(changes: changes, support: support) else { + return .alter( + try await DatabaseManager.shared.schemaChangeStatements( + tableName: tableName, + changes: changes, + scope: scope + ) + ) + } + return .rebuild( + try await StructureTableRebuildHandler.prepare(changes: changes, tableName: tableName, scope: scope) + ) + } + + func previewStagedChanges(coordinator: MainContentCoordinator) async { + let changes = changeManager.getChangesArray() + guard !changes.isEmpty else { return } + + let plan: StructureSavePlan + do { + plan = try await stagedSavePlan(for: changes) + } catch { + guard mayPresentPreview(on: coordinator) else { return } + coordinator.toolbarState.previewStatements = ["-- Error generating SQL: \(error.localizedDescription)"] + coordinator.activeSheet = .sqlPreview + return + } + + guard mayPresentPreview(on: coordinator) else { return } + switch plan { + case .alter: + coordinator.toolbarState.previewStatements = plan.displayStatements + coordinator.activeSheet = .sqlPreview + case .rebuild(let prepared): + coordinator.tableRebuildRequest = TableRebuildReviewRequest( + tableName: prepared.tableName, + scope: prepared.scope, + plan: prepared.plan, + action: nil + ) + coordinator.activeSheet = .tableRebuildReview + } + } + + private func mayPresentPreview(on coordinator: MainContentCoordinator) -> Bool { + guard coordinator.activeSheet == nil, + let selectedTab = coordinator.tabManager.selectedTab, + selectedTab.display.resultsViewMode == .structure + else { + return false + } + return coordinator.structureSessions[selectedTab.id] === self + } +} diff --git a/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift b/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift index 3d5c70dd4a..5d6a354371 100644 --- a/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift +++ b/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift @@ -75,8 +75,7 @@ extension TableStructureView { tableName: tableName, scope: prepared.scope, plan: prepared.plan, - actionTitle: String(localized: "Rebuild Table"), - perform: { + action: TableRebuildReviewRequest.Action(title: String(localized: "Rebuild Table")) { do { try await StructureColumnReorderHandler.execute( prepared, tableName: tableName, databaseType: connection.type diff --git a/TablePro/Views/Structure/TableStructureView+Schema.swift b/TablePro/Views/Structure/TableStructureView+Schema.swift index 153547daa6..048abcef48 100644 --- a/TablePro/Views/Structure/TableStructureView+Schema.swift +++ b/TablePro/Views/Structure/TableStructureView+Schema.swift @@ -30,24 +30,8 @@ extension TableStructureView { return } - guard let pluginDriver = (DatabaseManager.shared.driver(for: connection.id) as? PluginDriverAdapter)?.schemaPluginDriver else { - toolbarState.previewStatements = ["-- Error: no plugin driver available for DDL generation"] - coordinator?.activeSheet = .sqlPreview - return - } - - let generator = SchemaStatementGenerator( - tableName: tableName, - pluginDriver: pluginDriver - ) - - do { - let schemaStatements = try generator.generate(changes: changes) - toolbarState.previewStatements = schemaStatements.map(\.sql) - } catch { - toolbarState.previewStatements = ["-- Error generating SQL: \(error.localizedDescription)"] - } - coordinator?.activeSheet = .sqlPreview + guard let coordinator else { return } + Task { await session.previewStagedChanges(coordinator: coordinator) } } /// The part of a save only a mounted view can do: refetch the sub-tab the user is looking at diff --git a/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift b/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift index 7425ee7775..8dfa1c0450 100644 --- a/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift +++ b/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift @@ -75,6 +75,11 @@ private final class SchemaRoutingDriver: SchemaRoutingBaseDriver, PluginDatabase "ALTER TABLE \(qualified(table)) ADD COLUMN `\(column.name)` \(column.dataType)" } + func generateCreateTableSQL(definition: PluginCreateTableDefinition) -> String? { + let columns = definition.columns.map { "`\($0.name)` \($0.dataType)" }.joined(separator: ", ") + return "CREATE TABLE \(qualified(definition.tableName)) (\(columns))" + } + private func qualified(_ table: String) -> String { guard let schema, !schema.isEmpty else { return "`\(table)`" } return "`\(schema)`.`\(table)`" @@ -151,6 +156,19 @@ struct DatabaseManagerSchemaChangeRoutingTests { return pluginDriver } + private static func composeAndSave( + changes: [SchemaChange], + databaseType: DatabaseType, + scope: DatabaseScope + ) async throws { + let statements = try await DatabaseManager.shared.schemaChangeStatements( + tableName: "orders", + changes: changes, + scope: scope + ) + try await DatabaseManager.shared.executeSchemaChanges(statements, databaseType: databaseType, scope: scope) + } + private static func tearDown(_ connections: DatabaseConnection...) { for connection in connections { MetadataConnectionPool.shared.closeAll(connectionId: connection.id) @@ -193,8 +211,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { let scope = try #require(Self.makeScope(connectionB, database: "beta")) let pooledB = try await Self.seedPooledDriver(connectionB, scope: scope) - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: .mysql, scope: scope @@ -219,8 +236,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { let scope = try #require(Self.makeScope(connection, database: "orders")) let pooled = try await Self.seedPooledDriver(connection, scope: scope) - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: .mysql, scope: scope @@ -241,8 +257,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { defer { Self.tearDown(connection) } let scope = try #require(Self.makeScope(connection, database: "orders")) - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: Self.singleConnectionType, scope: scope @@ -265,8 +280,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { let scope = try #require(Self.makeScope(connection, database: "orders")) await #expect(throws: DatabaseError.self) { - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: Self.singleConnectionType, scope: scope @@ -286,8 +300,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { let scope = try #require(Self.makeScope(connection, database: "orders")) let pooled = try await Self.seedPooledDriver(connection, scope: scope) - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: .postgresql, scope: scope @@ -310,8 +323,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { let scope = try #require(Self.makeScope(connection, database: "orders", schema: "sales")) let pooled = try await Self.seedPooledDriver(connection, scope: scope) - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: .mssql, scope: scope @@ -338,8 +350,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { let scope = try #require(Self.makeScope(connection, database: "orders")) _ = try await Self.seedPooledDriver(connection, scope: scope) - try await DatabaseManager.shared.executeSchemaChanges( - tableName: "orders", + try await Self.composeAndSave( changes: [Self.makeAddColumnChange()], databaseType: .mysql, scope: scope @@ -350,6 +361,88 @@ struct DatabaseManagerSchemaChangeRoutingTests { #expect(broadcast.first?.scope == scope) #expect(broadcast.first?.scope?.database == "orders") } + + private static func invoicesDefinition() -> PluginCreateTableDefinition { + PluginCreateTableDefinition( + tableName: "invoices", + columns: [PluginColumnDefinition(name: "id", dataType: "INT")], + primaryKeyColumns: [] + ) + } + + @Test( + "The composer hands over a driver on the tab's schema while the session driver sits on another", + arguments: [true, false] + ) + func composerDriverSitsOnTheScopeSchema(pooled: Bool) async throws { + let type = pooled ? DatabaseType.mssql : Self.singleConnectionType + let (connection, driver) = Self.makeSession(type: type, savedDatabase: "orders", browseSchema: "dbo") + defer { Self.tearDown(connection) } + + let scope = try #require(Self.makeScope(connection, database: "orders", schema: "sales")) + if pooled { + _ = try await Self.seedPooledDriver(connection, scope: scope) + } + let composedOn = try await DatabaseManager.shared.withSchemaComposer( + scope: scope, + route: DatabaseManager.shared.schemaChangeRoute(for: scope) + ) { _, pluginDriver in + pluginDriver.currentSchema + } + + #expect(composedOn == "sales") + #expect(driver.currentSchema == (pooled ? "dbo" : "sales")) + } + + @Test("A Create Table draft is composed on the tab's schema and runs unchanged on its own connection") + func createTableDraftComposesOnTheScopeSchema() async throws { + let (connection, driver) = Self.makeSession(type: .mssql, savedDatabase: "orders", browseSchema: "dbo") + defer { Self.tearDown(connection) } + + let scope = try #require(Self.makeScope(connection, database: "orders", schema: "sales")) + let pooled = try await Self.seedPooledDriver(connection, scope: scope) + var column = EditableColumnDefinition.placeholder() + column.name = "id" + column.dataType = "INT" + let plan = CreateTableDraftBuilder.plan( + tableName: "invoices", + options: CreateTableOptions(), + columns: [column], + indexes: [], + foreignKeys: [], + dialect: ForeignKeyDialect.forType(.mssql), + includesEngineOptions: false + ) + + let composed = try await DatabaseManager.shared.createTableStatements(plan: plan, scope: scope) + try await DatabaseManager.shared.executeCreateTable( + statements: composed.statements, + databaseType: .mssql, + scope: scope + ) + + #expect(composed.statements == ["CREATE TABLE `sales`.`invoices` (`id` INT)"]) + #expect(pooled.executedQueries == composed.statements) + #expect(driver.executedQueries.isEmpty) + } + + @Test("An import's CREATE TABLE is composed on the route that runs it, pinned to the import's schema") + func importCreateTableComposesOnTheExecutionRoute() async throws { + let (connection, driver) = Self.makeSession(type: .postgresql, savedDatabase: "orders", browseSchema: "public") + defer { Self.tearDown(connection) } + + let scope = try #require(Self.makeScope(connection, database: "orders", schema: "sales")) + let route = DatabaseManager.shared.executionRoute(for: scope) + let statements = try await DatabaseManager.shared.createTableStatements( + definition: Self.invoicesDefinition(), + scope: scope, + route: route + ) + + #expect(route == .sessionDriver) + #expect(statements == ["CREATE TABLE `sales`.`invoices` (`id` INT)"]) + #expect(driver.currentSchema == "sales") + } } @MainActor diff --git a/TableProTests/Core/Database/SchemaCompositionGuardTests.swift b/TableProTests/Core/Database/SchemaCompositionGuardTests.swift new file mode 100644 index 0000000000..108abe1463 --- /dev/null +++ b/TableProTests/Core/Database/SchemaCompositionGuardTests.swift @@ -0,0 +1,60 @@ +// +// SchemaCompositionGuardTests.swift +// TableProTests +// + +import Foundation +import Testing + +@Suite("Schema composition guard") +struct SchemaCompositionGuardTests { + private static let appDirectory: URL = { + var url = URL(fileURLWithPath: #filePath) + for _ in 0 ..< 4 { + url.deleteLastPathComponent() + } + return url.appendingPathComponent("TablePro") + }() + + private static let composerEntryPoints = [ + "SchemaStatementGenerator(", + "CreateTableStatementComposer.compose(", + "generateCreateTableStatements(" + ] + + private static let scopedComposers: Set = [ + "DatabaseManager+SchemaComposition.swift", + "StructureTableRebuildHandler.swift" + ] + + private static let knownUnscopedComposers: Set = [ + "SchemaSyncScriptBuilder.swift" + ] + + private static func appSources() throws -> [(name: String, text: String)] { + guard let enumerator = FileManager.default.enumerator( + at: appDirectory, includingPropertiesForKeys: nil + ) else { return [] } + return try enumerator.compactMap { $0 as? URL } + .filter { $0.pathExtension == "swift" } + .map { ($0.lastPathComponent, try String(contentsOf: $0, encoding: .utf8)) } + } + + @Test("The scan reaches the app sources and the scoped composer") + func sourcesAreReachable() throws { + let sources = try Self.appSources() + #expect(sources.count > 100, "The app sources were not found; the guard below would pass vacuously") + #expect(sources.contains { $0.name == "DatabaseManager+SchemaComposition.swift" }) + } + + @Test("DDL is composed only on a driver leased for the statement's own scope") + func ddlIsComposedOnlyOnAScopedDriver() throws { + let allowed = Self.scopedComposers.union(Self.knownUnscopedComposers) + let offenders = try Self.appSources() + .filter { !allowed.contains($0.name) } + .filter { source in Self.composerEntryPoints.contains { source.text.contains($0) } } + .map(\.name) + + #expect(offenders.isEmpty, "These files compose DDL outside a scoped lease: \(offenders)") + } +} diff --git a/TableProTests/Core/SchemaTracking/PrimaryKeyConstraintLookupTests.swift b/TableProTests/Core/SchemaTracking/PrimaryKeyConstraintLookupTests.swift new file mode 100644 index 0000000000..617f855662 --- /dev/null +++ b/TableProTests/Core/SchemaTracking/PrimaryKeyConstraintLookupTests.swift @@ -0,0 +1,149 @@ +// +// PrimaryKeyConstraintLookupTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private class LookupBaseDriver { + var supportsTransactions: Bool { false } + 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 LookupDriver: LookupBaseDriver, PluginDatabaseDriver, @unchecked Sendable { + private(set) var executedQueries: [String] = [] + var answer: String? + var failure: Error? + let supportsSchemas: Bool + let currentSchema: String? + + init(currentSchema: String?) { + self.currentSchema = currentSchema + self.supportsSchemas = currentSchema != nil + super.init() + } + + func execute(query: String) async throws -> PluginQueryResult { + executedQueries.append(query) + if let failure { throw failure } + let rows: [[PluginCellValue]] = answer.map { [[.text($0)]] } ?? [] + return PluginQueryResult( + columns: ["CONSTRAINT_NAME"], columnTypeNames: ["TEXT"], rows: rows, rowsAffected: 0, executionTime: 0 + ) + } + + func switchDatabase(to database: String) async throws {} +} + +@Suite("Primary key constraint lookup") +@MainActor +struct PrimaryKeyConstraintLookupTests { + private static func adapter(_ driver: LookupDriver) -> PluginDriverAdapter { + PluginDriverAdapter( + connection: TestFixtures.makeConnection(type: .postgresql), + pluginDriver: driver + ) + } + + @Test("Asks the catalog for the key being dropped, in the driver's own schema") + func asksForTheKeyInTheDriverSchema() async { + let driver = LookupDriver(currentSchema: "reporting") + driver.answer = "orders_pkey" + + let name = await PrimaryKeyConstraintLookup.constraintName( + tableName: "sales", + changes: [.modifyPrimaryKey(old: ["id"], new: ["id", "region"])], + driver: Self.adapter(driver) + ) + + #expect(name == "orders_pkey") + #expect(driver.executedQueries.count == 1) + let query = driver.executedQueries.first ?? "" + #expect(query.contains("TABLE_SCHEMA = 'reporting'")) + #expect(query.contains("TABLE_NAME = 'sales'")) + #expect(query.contains("CONSTRAINT_TYPE = 'PRIMARY KEY'")) + } + + @Test("Escapes the table name as a string literal") + func escapesTheTableName() async { + let driver = LookupDriver(currentSchema: "public") + + _ = await PrimaryKeyConstraintLookup.constraintName( + tableName: "o'rders", + changes: [.modifyPrimaryKey(old: ["id"], new: [])], + driver: Self.adapter(driver) + ) + + #expect(driver.executedQueries.first?.contains("TABLE_NAME = 'o''rders'") == true) + } + + @Test("Asks nothing when no existing key is dropped") + func asksNothingWithoutADrop() async { + let driver = LookupDriver(currentSchema: "public") + + let added = await PrimaryKeyConstraintLookup.constraintName( + tableName: "sales", + changes: [.modifyPrimaryKey(old: [], new: ["id"])], + driver: Self.adapter(driver) + ) + let unrelated = await PrimaryKeyConstraintLookup.constraintName( + tableName: "sales", + changes: [.deleteColumn(EditableColumnDefinition.placeholder())], + driver: Self.adapter(driver) + ) + + #expect(added == nil) + #expect(unrelated == nil) + #expect(driver.executedQueries.isEmpty) + } + + @Test("Asks nothing on an engine without schemas") + func asksNothingWithoutSchemas() async { + let driver = LookupDriver(currentSchema: nil) + + let name = await PrimaryKeyConstraintLookup.constraintName( + tableName: "sales", + changes: [.modifyPrimaryKey(old: ["id"], new: ["id", "region"])], + driver: Self.adapter(driver) + ) + + #expect(name == nil) + #expect(driver.executedQueries.isEmpty) + } + + @Test("A catalog that cannot answer leaves the name unknown") + func failureLeavesTheNameUnknown() async { + let driver = LookupDriver(currentSchema: "public") + driver.failure = DatabaseError.queryFailed("relation does not exist") + + let name = await PrimaryKeyConstraintLookup.constraintName( + tableName: "sales", + changes: [.modifyPrimaryKey(old: ["id"], new: [])], + driver: Self.adapter(driver) + ) + + #expect(name == nil) + #expect(driver.executedQueries.count == 1) + } +} diff --git a/TableProTests/Views/Structure/CreateTableDraftCompositionTests.swift b/TableProTests/Views/Structure/CreateTableDraftCompositionTests.swift new file mode 100644 index 0000000000..a28cfe9301 --- /dev/null +++ b/TableProTests/Views/Structure/CreateTableDraftCompositionTests.swift @@ -0,0 +1,252 @@ +// +// CreateTableDraftCompositionTests.swift +// TableProTests +// + +import Combine +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private class CompositionBaseDriver { + var supportsSchemas: Bool { true } + var supportsTransactions: Bool { false } + 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 CompositionDriver: CompositionBaseDriver, PluginDatabaseDriver, @unchecked Sendable { + private(set) var createTableRequests = 0 + private var schema: String? + + var currentSchema: String? { schema } + + init(currentSchema: String?) { + self.schema = currentSchema + super.init() + } + + func execute(query: String) async throws -> PluginQueryResult { + PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func switchDatabase(to database: String) async throws {} + + func switchSchema(to schema: String) async throws { + self.schema = schema + } + + func generateCreateTableSQL(definition: PluginCreateTableDefinition) -> String? { + createTableRequests += 1 + let columns = definition.columns.map { "\"\($0.name)\" \($0.dataType)" }.joined(separator: ", ") + return "CREATE TABLE \"\(schema ?? "")\".\"\(definition.tableName)\" (\(columns))" + } +} + +@MainActor +private final class CompositionLatch { + private var waiters: [CheckedContinuation] = [] + private var isOpen = false + + func open() { + guard !isOpen else { return } + isOpen = true + let pending = waiters + waiters = [] + for waiter in pending { + waiter.resume() + } + } + + func wait() async { + guard !isOpen else { return } + await withCheckedContinuation { waiters.append($0) } + } +} + +@Suite("Create Table draft composition", .serialized) +@MainActor +struct CreateTableDraftCompositionTests { + private static func inject(type: DatabaseType, sessionSchema: String) -> (DatabaseConnection, CompositionDriver) { + let connection = TestFixtures.makeConnection(database: "shop", type: type) + let driver = CompositionDriver(currentSchema: sessionSchema) + var session = ConnectionSession( + connection: connection, + driver: PluginDriverAdapter(connection: connection, pluginDriver: driver) + ) + session.browseDatabase = "shop" + session.browseSchema = sessionSchema + DatabaseManager.shared.injectSession(session, for: connection.id) + return (connection, driver) + } + + private static func seedPooledDriver( + _ connection: DatabaseConnection, + scope: DatabaseScope + ) async throws -> CompositionDriver { + let driver = CompositionDriver(currentSchema: scope.schema) + let adapter = PluginDriverAdapter(connection: connection, pluginDriver: driver) + try await adapter.connect() + MetadataConnectionPool.shared.injectEntry(adapter, scope: scope) + return driver + } + + private static func tearDown(_ connection: DatabaseConnection) { + MetadataConnectionPool.shared.closeAll(connectionId: connection.id) + DatabaseManager.shared.removeSession(for: connection.id) + } + + private static func makeDraft(tableName: String) -> CreateTableDraft { + let draft = CreateTableDraft() + draft.tableName = tableName + var column = EditableColumnDefinition.placeholder() + column.name = "id" + column.dataType = "INT" + draft.changeManager.workingColumns = [column] + return draft + } + + @Test("A draft is composed on the tab's schema, not the one the session driver sits on") + func composesOnTheTabSchema() async throws { + let (connection, sessionDriver) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let scope = DatabaseScope(connectionId: connection.id, database: "shop", schema: "reporting") + _ = try await Self.seedPooledDriver(connection, scope: scope) + let draft = Self.makeDraft(tableName: "sales") + + await draft.recompose(databaseType: .postgresql, scope: scope) + + #expect(draft.composed?.statements == ["CREATE TABLE \"reporting\".\"sales\" (\"id\" INT)"]) + #expect(draft.compositionFailure == nil) + #expect(sessionDriver.createTableRequests == 0) + } + + @Test("The last composed statement stays on screen until the next one lands") + func keepsTheLastStatementUntilTheNextLands() async throws { + let (connection, _) = Self.inject(type: .pglite, sessionSchema: "reporting") + defer { Self.tearDown(connection) } + let scope = DatabaseScope(connectionId: connection.id, database: "shop", schema: "reporting") + let draft = Self.makeDraft(tableName: "sales") + await draft.recompose(databaseType: .pglite, scope: scope) + #expect(draft.composed?.tableName == "sales") + + let acquired = CompositionLatch() + let release = CompositionLatch() + let holder = Task { @MainActor in + try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess(connection.id) { + acquired.open() + await release.wait() + } + } + await acquired.wait() + + draft.tableName = "invoices" + let recomposition = Task { @MainActor in + await draft.recompose(databaseType: .pglite, scope: scope) + } + for _ in 0 ..< 5 { + await Task.yield() + } + + #expect(draft.composed?.tableName == "sales") + #expect(draft.composed?.statements == ["CREATE TABLE \"reporting\".\"sales\" (\"id\" INT)"]) + + release.open() + try await holder.value + await recomposition.value + + #expect(draft.composed?.statements == ["CREATE TABLE \"reporting\".\"invoices\" (\"id\" INT)"]) + } + + @Test("A composition that fails leaves the last good statement in place") + func failureKeepsTheLastStatement() async throws { + let (connection, _) = Self.inject(type: .pglite, sessionSchema: "reporting") + defer { Self.tearDown(connection) } + let scope = DatabaseScope(connectionId: connection.id, database: "shop", schema: "reporting") + let draft = Self.makeDraft(tableName: "sales") + await draft.recompose(databaseType: .pglite, scope: scope) + + DatabaseManager.shared.removeSession(for: connection.id) + draft.tableName = "invoices" + await draft.recompose(databaseType: .pglite, scope: scope) + + #expect(draft.composed?.statements == ["CREATE TABLE \"reporting\".\"sales\" (\"id\" INT)"]) + #expect(draft.compositionFailure != nil) + } + + @Test("An unchanged draft is not composed again") + func unchangedDraftIsNotComposedAgain() async throws { + let (connection, driver) = Self.inject(type: .pglite, sessionSchema: "reporting") + defer { Self.tearDown(connection) } + let scope = DatabaseScope(connectionId: connection.id, database: "shop", schema: "reporting") + let draft = Self.makeDraft(tableName: "sales") + + await draft.recompose(databaseType: .pglite, scope: scope) + await draft.recompose(databaseType: .pglite, scope: scope) + + #expect(driver.createTableRequests == 1) + } + + @Test( + "A grid edit reaches whoever observes the draft, so the preview and Create Table follow it", + arguments: [StructureTab.columns, StructureTab.indexes, StructureTab.foreignKeys] + ) + func gridEditPublishesTheDraft(tab: StructureTab) { + let connection = TestFixtures.makeConnection(type: .postgresql) + let scope = DatabaseScope(connectionId: connection.id, database: "shop", schema: "reporting") + let draft = Self.makeDraft(tableName: "sales") + draft.changeManager.addNewIndex() + draft.changeManager.addNewForeignKey() + let delegate = CreateTableGridDelegate( + structureChangeManager: draft.changeManager, + structureTab: tab, + connection: connection + ) + delegate.orderedFields = StructureRowProvider( + changeManager: draft.changeManager, + tab: tab, + databaseType: connection.type, + additionalFields: [.primaryKey], + serverSupport: .unrestricted + ).orderedColumnFields + let keyBefore = draft.compositionKey(scope: scope) + + var published = 0 + let observation = draft.objectWillChange.sink { published += 1 } + defer { observation.cancel() } + delegate.dataGridDidEditCell(row: 0, column: 0, newValue: "email") + + #expect(published > 0) + #expect(draft.compositionKey(scope: scope) != keyBefore) + } + + @Test("A draft with nothing to create names what is missing without a connection") + func emptyDraftNeedsNoConnection() async { + let scope = DatabaseScope(connectionId: UUID(), database: "shop", schema: nil) + let draft = CreateTableDraft() + + await draft.recompose(databaseType: .postgresql, scope: scope) + + #expect(draft.composed?.statements.isEmpty == true) + #expect(draft.composed?.issues.isEmpty == false) + #expect(draft.compositionFailure == nil) + } +} diff --git a/TableProTests/Views/Structure/StructureSavePlanTests.swift b/TableProTests/Views/Structure/StructureSavePlanTests.swift new file mode 100644 index 0000000000..66cff5102e --- /dev/null +++ b/TableProTests/Views/Structure/StructureSavePlanTests.swift @@ -0,0 +1,439 @@ +// +// StructureSavePlanTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private class SavePlanBaseDriver { + var supportsTransactions: Bool { false } + 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 SavePlanDriver: SavePlanBaseDriver, PluginDatabaseDriver, @unchecked Sendable { + private(set) var executedQueries: [String] = [] + var primaryKeyConstraints: [String: String] = [:] + var rebuildPlan: PluginColumnReorderPlan? + private let hasSchemas: Bool + private var schema: String? + + var supportsSchemas: Bool { hasSchemas } + var currentSchema: String? { schema } + + init(currentSchema: String?, hasSchemas: Bool = true) { + self.schema = currentSchema + self.hasSchemas = hasSchemas + super.init() + } + + func execute(query: String) async throws -> PluginQueryResult { + executedQueries.append(query) + let rows = primaryKeyConstraints.compactMap { table, name -> [PluginCellValue]? in + guard query.contains("INFORMATION_SCHEMA.TABLE_CONSTRAINTS"), + query.contains("TABLE_SCHEMA = '\(schema ?? "")'"), + query.contains("TABLE_NAME = '\(table)'") else { return nil } + return [.text(name)] + } + return PluginQueryResult( + columns: ["CONSTRAINT_NAME"], columnTypeNames: ["TEXT"], rows: rows, rowsAffected: 0, executionTime: 0 + ) + } + + func switchDatabase(to database: String) async throws {} + + 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)" + } + + func generateModifyPrimaryKeySQL( + table: String, + oldColumns: [String], + newColumns: [String], + constraintName: String? + ) -> [String]? { + var statements: [String] = [] + if !oldColumns.isEmpty { + let name = constraintName.map { "\"\($0)\"" } ?? "/* unknown constraint */" + statements.append("ALTER TABLE \(qualified(table)) DROP CONSTRAINT \(name)") + } + if !newColumns.isEmpty { + let columns = newColumns.map { "\"\($0)\"" }.joined(separator: ", ") + statements.append("ALTER TABLE \(qualified(table)) ADD PRIMARY KEY (\(columns))") + } + return statements + } + + func generateTableRebuildPlan( + table: String, + schema: String?, + respecification: PluginTableRespecification + ) async throws -> PluginColumnReorderPlan? { + rebuildPlan + } + + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? { + "fingerprint" + } + + private func qualified(_ table: String) -> String { + guard let schema, !schema.isEmpty else { return "\"\(table)\"" } + return "\"\(schema)\".\"\(table)\"" + } +} + +@Suite("Structure save plan", .serialized) +@MainActor +struct StructureSavePlanTests { + private static func inject( + type: DatabaseType, + database: String = "shop", + sessionSchema: String?, + hasSchemas: Bool = true + ) -> (DatabaseConnection, SavePlanDriver) { + let connection = TestFixtures.makeConnection(database: database, type: type) + let driver = SavePlanDriver(currentSchema: sessionSchema, hasSchemas: hasSchemas) + var session = ConnectionSession( + connection: connection, + driver: PluginDriverAdapter(connection: connection, pluginDriver: driver) + ) + session.browseDatabase = database + session.browseSchema = sessionSchema + DatabaseManager.shared.injectSession(session, for: connection.id) + return (connection, driver) + } + + private static func seedPooledDriver( + _ connection: DatabaseConnection, + scope: DatabaseScope + ) async throws -> SavePlanDriver { + let driver = SavePlanDriver(currentSchema: scope.schema) + let adapter = PluginDriverAdapter(connection: connection, pluginDriver: driver) + try await adapter.connect() + MetadataConnectionPool.shared.injectEntry(adapter, scope: scope) + return driver + } + + private static func tearDown(_ connection: DatabaseConnection) { + MetadataConnectionPool.shared.closeAll(connectionId: connection.id) + DatabaseManager.shared.removeSession(for: connection.id) + } + + private static func addColumn(named name: String = "notes") -> SchemaChange { + var column = EditableColumnDefinition.placeholder() + column.name = name + column.dataType = "TEXT" + return .addColumn(column) + } + + private static func completeForeignKey() -> EditableForeignKeyDefinition { + var foreignKey = EditableForeignKeyDefinition.placeholder() + foreignKey.name = "fk_artist" + foreignKey.columns = ["ArtistId"] + foreignKey.referencedTable = "Artist" + foreignKey.referencedColumns = ["ArtistId"] + return foreignKey + } + + private static func rebuildPlan(isRunnable: Bool) -> PluginColumnReorderPlan { + PluginColumnReorderPlan( + statements: [ + "CREATE TABLE \"_Album_new\" (\"AlbumId\" INTEGER, \"ArtistId\" INTEGER REFERENCES \"Artist\")", + "INSERT INTO \"_Album_new\" SELECT * FROM \"Album\"", + "DROP TABLE \"Album\"", + "ALTER TABLE \"_Album_new\" RENAME TO \"Album\"" + ], + prologue: ["PRAGMA foreign_keys = off"], + epilogue: ["PRAGMA foreign_keys = on"], + isTransactional: true, + cost: .tableRebuild, + caveats: ["Triggers on Album are recreated from their stored text."], + isRunnable: isRunnable, + verifications: [] + ) + } + + private static func makeStructureCoordinator( + _ connection: DatabaseConnection, + session: StructureEditingSession, + viewMode: ResultsViewMode = .structure + ) -> MainContentCoordinator { + let tabManager = QueryTabManager() + let coordinator = MainContentCoordinator( + connection: connection, + tabManager: tabManager, + changeManager: DataChangeManager(), + toolbarState: ConnectionToolbarState() + ) + let tab = QueryTab( + title: session.tableName, + query: "SELECT * FROM \(session.tableName)", + tabType: .table, + tableName: session.tableName + ) + tabManager.tabs = [tab] + tabManager.selectedTabId = tab.id + tabManager.tabs[0].display.resultsViewMode = viewMode + coordinator.structureSessions[tab.id] = session + return coordinator + } + + private static func stage(_ foreignKey: EditableForeignKeyDefinition, on session: StructureEditingSession) { + session.changeManager.loadSchema( + tableName: session.tableName, + columns: [ + TestFixtures.makeColumnInfo(name: "AlbumId", dataType: "INTEGER"), + TestFixtures.makeColumnInfo(name: "ArtistId", dataType: "INTEGER", isNullable: true, isPrimaryKey: false) + ], + indexes: [], + foreignKeys: [], + primaryKey: ["AlbumId"] + ) + session.changeManager.addForeignKey(foreignKey) + } + + private static func stageAColumn(on session: StructureEditingSession) { + let manager = session.changeManager + manager.loadSchema(tableName: session.tableName, columns: [], indexes: [], foreignKeys: [], primaryKey: []) + manager.addNewColumn() + guard var column = manager.workingColumns.last else { return } + column.name = "notes" + column.dataType = "TEXT" + manager.updateColumn(id: column.id, with: column) + } + + // MARK: - Scope + + @Test("The plan names the tab's schema on a pooled connection, not the session driver's") + func pooledPlanNamesTheTabSchema() async throws { + let (connection, sessionDriver) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + let pooled = try await Self.seedPooledDriver(connection, scope: session.scope) + + let plan = try await session.stagedSavePlan(for: [Self.addColumn()]) + + #expect(plan.displayStatements == ["ALTER TABLE \"reporting\".\"orders\" ADD COLUMN \"notes\" TEXT;"]) + #expect(sessionDriver.executedQueries.isEmpty) + #expect(pooled.currentSchema == "reporting") + } + + @Test("An engine with one connection composes on the session driver after pinning it to the tab's schema") + func singleConnectionPlanPinsTheTabSchema() async throws { + let (connection, sessionDriver) = Self.inject(type: .pglite, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + + let plan = try await session.stagedSavePlan(for: [Self.addColumn()]) + + #expect(plan.displayStatements == ["ALTER TABLE \"reporting\".\"orders\" ADD COLUMN \"notes\" TEXT;"]) + #expect(sessionDriver.currentSchema == "reporting") + } + + @Test("Save runs exactly the statements the plan showed") + func saveRunsThePlannedStatements() async throws { + let (connection, sessionDriver) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + let pooled = try await Self.seedPooledDriver(connection, scope: session.scope) + Self.stageAColumn(on: session) + + let planned = try await session.stagedSavePlan(for: session.changeManager.getChangesArray()) + let outcome = await session.applyStagedChanges(coordinator: nil) + + #expect(outcome == .applied) + #expect(pooled.executedQueries == planned.displayStatements) + #expect(sessionDriver.executedQueries.isEmpty) + } + + // MARK: - Primary key name + + @Test( + "A primary key change drops the constraint by the name the server reports", + arguments: [DatabaseType.postgresql, DatabaseType.pglite] + ) + func primaryKeyDropUsesTheServerName(type: DatabaseType) async throws { + let (connection, sessionDriver) = Self.inject(type: type, sessionSchema: "reporting") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "sales" + ) + sessionDriver.primaryKeyConstraints = ["sales": "orders_pkey"] + if DatabaseManager.shared.schemaChangeRoute(for: session.scope) == .pooled { + let pooled = try await Self.seedPooledDriver(connection, scope: session.scope) + pooled.primaryKeyConstraints = ["sales": "orders_pkey"] + } + + let plan = try await session.stagedSavePlan(for: [.modifyPrimaryKey(old: ["id"], new: ["id", "region"])]) + + #expect(plan.displayStatements.contains("ALTER TABLE \"reporting\".\"sales\" DROP CONSTRAINT \"orders_pkey\";")) + #expect(!plan.displayStatements.contains { $0.contains("unknown constraint") }) + } + + // MARK: - Rebuild + + @Test("A foreign key change on SQLite plans the rebuild Save would review") + func sqliteForeignKeyPlansTheRebuild() async throws { + let (connection, sessionDriver) = Self.inject( + type: .sqlite, database: "main", sessionSchema: nil, hasSchemas: false + ) + defer { Self.tearDown(connection) } + sessionDriver.rebuildPlan = Self.rebuildPlan(isRunnable: true) + let session = TestFixtures.makeStructureSession(connection: connection, database: "main", table: "Album") + + let plan = try await session.stagedSavePlan(for: [.addForeignKey(Self.completeForeignKey())]) + + guard case .rebuild(let prepared) = plan else { + Issue.record("A SQLite foreign key change must plan a rebuild, got \(plan)") + return + } + #expect(plan.displayStatements == Self.rebuildPlan(isRunnable: true).scriptStatements) + #expect(prepared.scope == session.scope) + } + + // MARK: - Preview + + @Test("Preview shows the ALTER statements Save would run") + func previewShowsTheAlterStatements() async throws { + let (connection, _) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + _ = try await Self.seedPooledDriver(connection, scope: session.scope) + Self.stageAColumn(on: session) + let coordinator = Self.makeStructureCoordinator(connection, session: session) + defer { coordinator.teardown() } + + await session.previewStagedChanges(coordinator: coordinator) + + #expect(coordinator.activeSheet?.id == ActiveSheet.sqlPreview.id) + #expect( + coordinator.toolbarState.previewStatements + == ["ALTER TABLE \"reporting\".\"orders\" ADD COLUMN \"notes\" TEXT;"] + ) + } + + @Test( + "Preview shows a rebuild read-only with its caveats, and offers to run nothing Save would refuse", + arguments: [true, false] + ) + func previewShowsTheRebuildReadOnly(isRunnable: Bool) async throws { + let (connection, sessionDriver) = Self.inject( + type: .sqlite, database: "main", sessionSchema: nil, hasSchemas: false + ) + defer { Self.tearDown(connection) } + let plan = Self.rebuildPlan(isRunnable: isRunnable) + sessionDriver.rebuildPlan = plan + let session = TestFixtures.makeStructureSession(connection: connection, database: "main", table: "Album") + Self.stage(Self.completeForeignKey(), on: session) + let coordinator = Self.makeStructureCoordinator(connection, session: session) + defer { coordinator.teardown() } + + await session.previewStagedChanges(coordinator: coordinator) + + #expect(coordinator.activeSheet?.id == ActiveSheet.tableRebuildReview.id) + let preview = try #require(coordinator.tableRebuildRequest) + #expect(preview.scriptStatements == plan.scriptStatements) + #expect(preview.warning == plan.caveats.joined(separator: " ")) + #expect(preview.isRunnable == isRunnable) + #expect(preview.runnableAction == nil) + + coordinator.activeSheet = nil + coordinator.tableRebuildRequest = nil + #expect(await session.applyStagedChanges(coordinator: coordinator) == .refused) + + let review = try #require(coordinator.tableRebuildRequest) + #expect(review.scriptStatements == preview.scriptStatements) + #expect(review.warning == preview.warning) + #expect((review.runnableAction != nil) == isRunnable) + } + + @Test("A preview that finishes after the tab left Structure presents nothing") + func previewAfterLeavingStructurePresentsNothing() async throws { + let (connection, _) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + _ = try await Self.seedPooledDriver(connection, scope: session.scope) + Self.stageAColumn(on: session) + let coordinator = Self.makeStructureCoordinator(connection, session: session, viewMode: .data) + defer { coordinator.teardown() } + + await session.previewStagedChanges(coordinator: coordinator) + + #expect(coordinator.activeSheet == nil) + #expect(coordinator.toolbarState.previewStatements.isEmpty) + } + + @Test("A preview never replaces a sheet that is already up") + func previewLeavesAnotherSheetAlone() async throws { + let (connection, _) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + _ = try await Self.seedPooledDriver(connection, scope: session.scope) + Self.stageAColumn(on: session) + let coordinator = Self.makeStructureCoordinator(connection, session: session) + defer { coordinator.teardown() } + coordinator.toolbarState.previewStatements = ["UPDATE \"orders\" SET \"total\" = 1;"] + coordinator.activeSheet = .sqlPreview + + await session.previewStagedChanges(coordinator: coordinator) + + #expect(coordinator.toolbarState.previewStatements == ["UPDATE \"orders\" SET \"total\" = 1;"]) + } + + @Test("A preview for a session the selected tab no longer owns presents nothing") + func previewForAnotherTabsSessionPresentsNothing() async throws { + let (connection, _) = Self.inject(type: .postgresql, sessionSchema: "public") + defer { Self.tearDown(connection) } + let session = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + _ = try await Self.seedPooledDriver(connection, scope: session.scope) + Self.stageAColumn(on: session) + let replacement = TestFixtures.makeStructureSession( + connection: connection, database: "shop", schema: "reporting", table: "orders" + ) + let coordinator = Self.makeStructureCoordinator(connection, session: replacement) + defer { coordinator.teardown() } + + await session.previewStagedChanges(coordinator: coordinator) + + #expect(coordinator.activeSheet == nil) + #expect(coordinator.toolbarState.previewStatements.isEmpty) + } +} diff --git a/TableProUITests/StructurePreviewSQLUITests.swift b/TableProUITests/StructurePreviewSQLUITests.swift new file mode 100644 index 0000000000..c05c3dd754 --- /dev/null +++ b/TableProUITests/StructurePreviewSQLUITests.swift @@ -0,0 +1,73 @@ +// +// StructurePreviewSQLUITests.swift +// TableProUITests +// + +import XCTest + +final class StructurePreviewSQLUITests: UITestCase { + func testPreviewingAForeignKeyDropOnSQLiteShowsTheRebuildSaveWouldReview() throws { + let app = try launchWithSampleDatabase() + let window = app.windows.firstMatch + + let grid = try openForeignKeysGrid(app: app, window: window) + gridPoint(in: grid, of: window, dy: 40).click() + XCTAssertTrue( + waitForPredicate(timeout: 10) { grid.tableRows.allElementsBoundByIndex.contains { $0.isSelected } }, + "The click must select Album's foreign key row" + ) + + let remove = window.buttons["structure-footer-remove"].firstMatch + XCTAssertTrue(remove.waitToExist(timeout: 20), "The Foreign Keys tab must offer a remove button") + XCTAssertTrue( + waitForPredicate(timeout: 10) { remove.isEnabled }, + "SQLite rebuilds the table to drop a foreign key, so removing one must be offered" + ) + remove.click() + + app.typeKey("p", modifierFlags: [.command, .shift]) + + let sheet = window.sheets.firstMatch + XCTAssertTrue(sheet.waitToExist(timeout: 30), "Preview SQL must open a sheet") + XCTAssertTrue( + sheet.buttons["Open in Query Editor"].firstMatch.waitToExist(timeout: 30), + "A foreign key change on SQLite is a table rebuild, and Preview must show that script" + ) + XCTAssertFalse( + sheet.buttons["sql-review-execute"].firstMatch.exists, + "Preview shows the rebuild and runs nothing" + ) + let text = sheet.staticTexts.allElementsBoundByIndex + .map { ($0.value as? String) ?? $0.label } + .joined(separator: " ") + XCTAssertFalse( + text.contains("Unsupported schema operation"), + "Preview must not refuse a change Save knows how to make, got: \(text)" + ) + } + + private func openForeignKeysGrid(app: XCUIApplication, window: XCUIElement) throws -> XCUIElement { + let row = objectBrowserRow("Album", in: window) + XCTAssertTrue(row.waitToExist(timeout: 20), "The object browser must list Album") + clickAtCenter(row) + + showStructure(in: app, window: window) + + let foreignKeys = window.radioGroups["structure-tab-picker"].firstMatch + .radioButtons + .matching(NSPredicate(format: "label BEGINSWITH %@", "Foreign Keys")) + .firstMatch + XCTAssertTrue(foreignKeys.waitToExist(timeout: 20), "SQLite has foreign keys, so the tab must be offered") + foreignKeys.click() + + let grid = window.tables.matching(identifier: "data-grid").firstMatch + XCTAssertTrue(grid.waitToExist(timeout: 30), "The structure editor must draw its foreign key grid") + XCTAssertTrue( + waitForPredicate(timeout: 30) { + grid.frame.width > 0 && grid.frame.height > 0 && grid.tableRows.count == 1 + }, + "Album references Artist and nothing else, so the grid must list exactly one foreign key" + ) + return grid + } +} diff --git a/docs/features/table-structure.mdx b/docs/features/table-structure.mdx index 1fb99c5964..fd4024ee21 100644 --- a/docs/features/table-structure.mdx +++ b/docs/features/table-structure.mdx @@ -168,7 +168,7 @@ SQLite is the one engine where listing and editing part company. The tab appears [Change Tracking](/features/change-tracking) covers the queue, undo (`Cmd+Z`) and redo (`Cmd+Shift+Z`). A save runs on the tab's own connection, database, and schema, the ones it was opened on, and never moves the sidebar or the toolbar. - **Save Changes** (`Cmd+S` or the toolbar checkmark) applies the queue. Changes that can lose data, dropping a column, changing a type, adding NOT NULL, changing the primary key, first show a confirmation listing each one. -- **Preview SQL** (`Cmd+Shift+P`) shows the generated statements without executing them. +- **Preview SQL** (`Cmd+Shift+P`) shows the statements **Save Changes** would run, on the tab's own schema, and runs none of them. A save that recreates the table previews as its rebuild script, with what the rebuild cannot carry over. The queue outlives everything short of an explicit discard: closing the tab, closing the window, quitting, and **Refresh** all ask first. A save that never reaches the server leaves the tab open with its queue intact.