Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Table maintenance from the object browser (OPTIMIZE, ANALYZE, CHECK, REPAIR, and VACUUM on PostgreSQL) now runs against the database the table you picked lives in. It ran on whichever database the connection happened to be on, which a tab from another database moves for the length of its query, so the command could maintain the same-named table in the wrong database and still report success.
- A table that exists in two databases can be opened in both. The object browser marked the row for a table you had open in another database, so clicking it did nothing at all: the row was already selected and the click changed nothing. The browser now marks a row only while the tab you are looking at belongs to the database on screen. (#2217)
- Server Dashboard, Users & Roles, Query Insights, ER Diagram and a linked SQL file now select the tab they already opened instead of doing nothing when another tab is in front. (#2217)
- The empty Favorites sidebar fits its width on macOS 15. The description and the New Favorite, New Folder and Link a Folder buttons ran past both edges of the sidebar and were cut off, so the buttons could not be read or reached. They now wrap and stack inside the sidebar.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,43 @@ extension MainContentCoordinator {
return driver.supportedMaintenanceOperations() ?? []
}

func showMaintenanceSheet(operation: String, tableName: String) {
activeSheet = .maintenance(operation: operation, tableName: tableName)
func showMaintenanceSheet(
operation: String,
tableName: String,
database: String? = nil,
schema: String? = nil
) {
activeSheet = .maintenance(
operation: operation, tableName: tableName, database: database, schema: schema
)
}

func executeMaintenance(operation: String, tableName: String, options: [String: String]) {
/// Runs against the database the object it names lives in, on a scoped lease.
///
/// A maintenance statement names its table and nothing else, so where it lands is decided
/// entirely by the connection's current database. Executing on the session driver directly left
/// that to chance: a cross-database tab pins the shared handle to its own database for the
/// length of its query and deliberately writes no session state back, so `OPTIMIZE TABLE
/// role_ability` could optimize the copy in another database while the sheet reported success.
/// Every other statement the user owns takes a scoped lease; this one now does too, which also
/// puts it behind the same gate rather than interleaving with a tab's work on one handle.
func executeMaintenance(
operation: String,
tableName: String,
options: [String: String],
database: String? = nil,
schema: String? = nil
) {
guard let driver = DatabaseManager.shared.driver(for: connectionId) else { return }
guard let statements = driver.maintenanceStatements(
operation: operation, table: tableName, options: options
) else { return }
/// The object the user picked names its own database, and only a command that names none
/// falls back to where the browser is pointing. `resolvedScope` is what decides that, so a
/// schema is never carried across a database boundary.
guard let scope = services.databaseManager.resolvedScope(
database: database, schema: schema, for: connectionId
) ?? browseScope else { return }

Task { [weak self] in
guard let self else { return }
Expand All @@ -245,8 +273,18 @@ extension MainContentCoordinator {
}
do {
var lastResult: QueryResult?
let route = DatabaseManager.shared.executionRoute(for: scope)
for sql in statements {
lastResult = try await driver.execute(query: sql)
/// `.protectedWrite`: a half-applied OPTIMIZE or REPAIR cannot be undone by
/// retrying, so the lease is registered to mark the connection busy and is never
/// reachable by Stop.
lastResult = try await DatabaseManager.shared.withScopedDriver(
scope: scope,
route: route,
cancellation: .protectedWrite
) { scopedDriver in
try await scopedDriver.execute(query: sql)
}
}
await AlertHelper.showInfoSheet(
title: String(format: String(localized: "%@ completed"), operation),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ extension MainContentCommandActions {
return coordinator?.supportedMaintenanceOperations() ?? []
}

/// The menu acts on the object browser's selection, and `TableInfo` carries a schema but no
/// database, so this names only the schema and the command falls back to the database being
/// browsed. The sidebar's own contextual menu carries the clicked row's database and does not.
func runMaintenanceOperation(_ operation: String) {
guard let object = selectedObject else { return }
coordinator?.showMaintenanceSheet(operation: operation, tableName: object.name)
coordinator?.showMaintenanceSheet(
operation: operation, tableName: object.name, schema: object.schema
)
}

var canCreateDatabase: Bool {
Expand Down
9 changes: 7 additions & 2 deletions TablePro/Views/Main/MainContentCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ enum ActiveSheet: Identifiable {
case exportQueryResults
case backupDatabase
case restoreDatabase(fileURL: URL)
case maintenance(operation: String, tableName: String)
/// The object's own database and schema travel with the request. A maintenance statement names
/// its table and nothing else, so acting on wherever the object browser happens to point
/// maintains the same-named table in another database whenever the two have drifted apart.
/// This is the rule the sidebar's other destructive commands already keep by carrying their ref.
case maintenance(operation: String, tableName: String, database: String?, schema: String?)
case createDatabase

var id: String {
Expand All @@ -64,7 +68,8 @@ enum ActiveSheet: Identifiable {
case .exportQueryResults: "exportQueryResults"
case .backupDatabase: "backupDatabase"
case .restoreDatabase(let fileURL): "restoreDatabase-\(fileURL.path)"
case .maintenance(let operation, let tableName): "maintenance-\(operation)-\(tableName)"
case .maintenance(let operation, let tableName, let database, let schema):
"maintenance-\(operation)-\(database ?? "")-\(schema ?? "")-\(tableName)"
case .createDatabase: "createDatabase"
}
}
Expand Down
12 changes: 10 additions & 2 deletions TablePro/Views/Main/MainContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,20 @@ struct MainContentView: View {
?? connection.database,
sourceURL: fileURL
)
case .maintenance(let operation, let tableName):
case .maintenance(let operation, let tableName, let database, let schema):
MaintenanceSheet(
operation: operation,
tableName: tableName,
databaseType: connection.type,
onExecute: coordinator.executeMaintenance
onExecute: { operation, tableName, options in
coordinator.executeMaintenance(
operation: operation,
tableName: tableName,
options: options,
database: database,
schema: schema
)
}
)
case .sqlPreview:
SQLReviewSheet(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@ extension DatabaseTreeOutlineCoordinator {
}
case .maintenance(let operation, let tableName, let ref):
activateThen(ref) { [weak self] in
self?.mainCoordinator?.showMaintenanceSheet(operation: operation, tableName: tableName)
self?.mainCoordinator?.showMaintenanceSheet(
operation: operation,
tableName: tableName,
database: ref.database,
schema: ref.schema
)
}
case .truncateTables(let names, let ref):
activateThen(ref) { [weak self] in
Expand Down
158 changes: 158 additions & 0 deletions TableProTests/Core/Database/ScopedDriverPinningTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
//
// ScopedDriverPinningTests.swift
// TableProTests
//
// What a scoped lease guarantees, and what running on the session driver directly does not.
// A maintenance statement names its table and nothing else, so the connection's current database
// decides where it lands, and nothing tracks where that is.
//

import Foundation
@testable import TablePro
import TableProPluginKit
import Testing

@Suite("Scoped driver pinning", .serialized)
@MainActor
struct ScopedDriverPinningTests {
private static func seed(
browseDatabase: String
) -> (connection: DatabaseConnection, recorder: CallRecordingPluginDriver) {
let connection = TestFixtures.makeConnection(database: browseDatabase, type: .mysql)
let recorder = CallRecordingPluginDriver()
var session = ConnectionSession(
connection: connection,
driver: PluginDriverAdapter(connection: connection, pluginDriver: recorder)
)
session.browseDatabase = browseDatabase
session.status = .connected
DatabaseManager.shared.injectSession(session, for: connection.id)
return (connection, recorder)
}

@Test("A scoped statement moves the connection onto its own database before it runs")
func pinsBeforeRunningTheStatement() async throws {
let (connection, recorder) = Self.seed(browseDatabase: "app")
defer { DatabaseManager.shared.removeSession(for: connection.id) }

let scope = DatabaseScope(connectionId: connection.id, database: "logs", schema: nil)
try await DatabaseManager.shared.withScopedDriver(
scope: scope,
route: DatabaseManager.shared.executionRoute(for: scope),
cancellation: .protectedWrite
) { driver in
_ = try await driver.execute(query: "OPTIMIZE TABLE `role_ability`")
}

#expect(recorder.calls == ["switch:logs", "execute:OPTIMIZE TABLE `role_ability`"])
}

/// The half that made an unscoped statement dangerous. A lease moves the shared connection and
/// deliberately writes no session state back, so the browse cursor still says `app` while the
/// handle sits on `logs`. Anything that then runs on the session driver without a scope of its
/// own inherits `logs` and reports success against the wrong database.
@Test("A lease leaves the shared connection on its database and the browse cursor untouched")
func leavesTheConnectionWhereTheLeasePutIt() async throws {
let (connection, recorder) = Self.seed(browseDatabase: "app")
defer { DatabaseManager.shared.removeSession(for: connection.id) }

let scope = DatabaseScope(connectionId: connection.id, database: "logs", schema: nil)
try await DatabaseManager.shared.withScopedDriver(
scope: scope,
route: DatabaseManager.shared.executionRoute(for: scope),
cancellation: .protectedWrite
) { driver in
_ = try await driver.execute(query: "SELECT 1")
}

#expect(recorder.currentDatabase == "logs")
#expect(DatabaseManager.shared.session(for: connection.id)?.resolvedBrowseDatabase == "app")
}

@Test("A statement scoped to the browsed database is pinned back onto it")
func pinsBackOntoTheBrowsedDatabase() async throws {
let (connection, recorder) = Self.seed(browseDatabase: "app")
defer { DatabaseManager.shared.removeSession(for: connection.id) }

let foreign = DatabaseScope(connectionId: connection.id, database: "logs", schema: nil)
try await DatabaseManager.shared.withScopedDriver(
scope: foreign,
route: DatabaseManager.shared.executionRoute(for: foreign),
cancellation: .cancellableRead
) { driver in
_ = try await driver.execute(query: "SELECT 1")
}

let browsed = try #require(DatabaseManager.shared.browseScope(for: connection.id))
recorder.reset()
try await DatabaseManager.shared.withScopedDriver(
scope: browsed,
route: DatabaseManager.shared.executionRoute(for: browsed),
cancellation: .protectedWrite
) { driver in
_ = try await driver.execute(query: "OPTIMIZE TABLE `role_ability`")
}

#expect(recorder.calls == ["switch:app", "execute:OPTIMIZE TABLE `role_ability`"])
#expect(recorder.currentDatabase == "app")
}
}

/// Records the order of the two calls that decide where a statement lands.
private final class CallRecordingPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
private let lock = NSLock()
private var recorded: [String] = []
private var database: String?

var calls: [String] {
lock.lock()
defer { lock.unlock() }
return recorded
}

var currentDatabase: String? {
lock.lock()
defer { lock.unlock() }
return database
}

func reset() {
lock.lock()
recorded = []
lock.unlock()
}

private func record(_ call: String) {
lock.lock()
recorded.append(call)
lock.unlock()
}

func switchDatabase(to database: String) async throws {
lock.lock()
self.database = database
lock.unlock()
record("switch:\(database)")
}

func execute(query: String) async throws -> PluginQueryResult {
record("execute:\(query)")
return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0)
}

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)
}
}
63 changes: 63 additions & 0 deletions TableProTests/Views/Main/MaintenanceSheetIdentityTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//
// MaintenanceSheetIdentityTests.swift
// TableProTests
//
// A maintenance request is identified by the object it names, database included. Without the
// database it is the same request in every database that holds a table by that name, which is
// how the command came to run against whichever one the connection happened to be on.
//

import Foundation
@testable import TablePro
import Testing

@Suite("Maintenance sheet identity")
struct MaintenanceSheetIdentityTests {
@Test("The same table in two databases is two different requests")
func distinguishesTwoDatabases() {
let online = ActiveSheet.maintenance(
operation: "OPTIMIZE TABLE", tableName: "role_ability", database: "banshi_online", schema: nil
)
let test = ActiveSheet.maintenance(
operation: "OPTIMIZE TABLE", tableName: "role_ability", database: "banshi_test", schema: nil
)

#expect(online.id != test.id)
}

@Test("The same table in two schemas of one database is two different requests")
func distinguishesTwoSchemas() {
let publicSchema = ActiveSheet.maintenance(
operation: "VACUUM", tableName: "orders", database: "app", schema: "public"
)
let reporting = ActiveSheet.maintenance(
operation: "VACUUM", tableName: "orders", database: "app", schema: "reporting"
)

#expect(publicSchema.id != reporting.id)
}

@Test("The same object is the same request")
func matchesTheSameObject() {
let first = ActiveSheet.maintenance(
operation: "ANALYZE TABLE", tableName: "orders", database: "app", schema: "public"
)
let second = ActiveSheet.maintenance(
operation: "ANALYZE TABLE", tableName: "orders", database: "app", schema: "public"
)

#expect(first.id == second.id)
}

@Test("A request that names no database is not the same as one that does")
func distinguishesAnUnnamedDatabase() {
let named = ActiveSheet.maintenance(
operation: "OPTIMIZE TABLE", tableName: "orders", database: "app", schema: nil
)
let unnamed = ActiveSheet.maintenance(
operation: "OPTIMIZE TABLE", tableName: "orders", database: nil, schema: nil
)

#expect(named.id != unnamed.id)
}
}
Loading