diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bc41c568..2fe3650bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,8 +37,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The filter panel's "Unset" button is now "Clear". It keeps your filter rows and only drops the applied state. To remove the rows, use "Remove All Filters" in the filter options menu. - A row's right-click menu now has "Apply Only This Filter". The inline per-row Apply button is gone. +### Changed + +- Expanding a database in the tree sidebar loads tables first and fills in procedures and functions in the background, so the table list appears after one round-trip instead of waiting for three queries to finish in sequence. + ### Fixed +- Expanding or collapsing a database or schema in the tree sidebar while its tables were still loading could crash the app. The tree now updates its rows without rebuilding the outline structure. +- MongoDB filters on `_id` and other ObjectId fields now match. A 24-character hex value is matched as an ObjectId as well as a string, so filtering by `_id` returns the row instead of nothing. (#1682) - Shift+Arrow in the data grid now starts and extends a cell selection from the focused cell. Cmd+Shift+Arrow extends to the row or column edge. - Delete key now removes all rows covered by a cell-range selection instead of ignoring it. - Right-clicking inside a multi-row or cell-range selection no longer collapses the selection first. diff --git a/TablePro/Core/Services/Query/DatabaseTreeMetadataService.swift b/TablePro/Core/Services/Query/DatabaseTreeMetadataService.swift index 29513ffbb..d1dd93e77 100644 --- a/TablePro/Core/Services/Query/DatabaseTreeMetadataService.swift +++ b/TablePro/Core/Services/Query/DatabaseTreeMetadataService.swift @@ -23,18 +23,15 @@ final class DatabaseTreeMetadataService { let schema: String? } - struct SchemaObjects: Equatable, Sendable { - var tables: [TableInfo] - var routines: [RoutineInfo] - } - private(set) var databaseList: [UUID: MetadataLoadState<[DatabaseMetadata]>] = [:] private(set) var schemaList: [DatabaseKey: MetadataLoadState<[String]>] = [:] - private(set) var objects: [ObjectsKey: MetadataLoadState] = [:] + private(set) var tablesState: [ObjectsKey: MetadataLoadState<[TableInfo]>] = [:] + private(set) var routinesState: [ObjectsKey: MetadataLoadState<[RoutineInfo]>] = [:] @ObservationIgnored private let databaseDedup = OnceTask() @ObservationIgnored private let schemaDedup = OnceTask() - @ObservationIgnored private let objectsDedup = OnceTask() + @ObservationIgnored private let tablesDedup = OnceTask() + @ObservationIgnored private let routinesDedup = OnceTask() @ObservationIgnored private static let logger = Logger( subsystem: "com.TablePro", category: "SidebarTree" @@ -60,16 +57,20 @@ final class DatabaseTreeMetadataService { schemaList[DatabaseKey(connectionId: connectionId, database: database)]?.value ?? [] } - func objectsState(connectionId: UUID, database: String, schema: String?) -> MetadataLoadState { - objects[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)] ?? .idle + func tablesLoadState(connectionId: UUID, database: String, schema: String?) -> MetadataLoadState<[TableInfo]> { + tablesState[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)] ?? .idle + } + + func routinesLoadState(connectionId: UUID, database: String, schema: String?) -> MetadataLoadState<[RoutineInfo]> { + routinesState[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)] ?? .idle } func tables(connectionId: UUID, database: String, schema: String?) -> [TableInfo] { - objects[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)]?.value?.tables ?? [] + tablesState[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)]?.value ?? [] } func routines(connectionId: UUID, database: String, schema: String?) -> [RoutineInfo] { - objects[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)]?.value?.routines ?? [] + routinesState[Self.objectsKey(connectionId: connectionId, database: database, schema: schema)]?.value ?? [] } // MARK: - Loads @@ -122,34 +123,61 @@ final class DatabaseTreeMetadataService { } } - func loadObjects(connectionId: UUID, database: String, schema: String?) async { + func loadTables(connectionId: UUID, database: String, schema: String?) async { guard isConnected(connectionId) else { return } let key = Self.objectsKey(connectionId: connectionId, database: database, schema: schema) - switch objects[key] ?? .idle { + switch tablesState[key] ?? .idle { case .loaded, .loading: return case .idle, .failed: break } - objects[key] = .loading + tablesState[key] = .loading let normalizedSchema = key.schema do { - let result = try await objectsDedup.execute(key: key) { [self] in + let list = try await tablesDedup.execute(key: key) { [self] in try await withDriver(connectionId: connectionId, database: database) { driver in - async let tables = driver.fetchTables(schema: normalizedSchema) - async let procedures = driver.fetchProcedures(schema: normalizedSchema) - async let functions = driver.fetchFunctions(schema: normalizedSchema) - return SchemaObjects( - tables: try await tables, - routines: try await procedures + functions - ) + try await driver.fetchTables(schema: normalizedSchema) + } + } + tablesState[key] = .loaded(list) + } catch is CancellationError { + if case .loading = tablesState[key] { tablesState[key] = .idle } + } catch { + tablesState[key] = .failed(error.localizedDescription) + Self.logger.warning( + "tables load failed db=\(database, privacy: .public) schema=\(schema ?? "nil", privacy: .public) error=\(error.localizedDescription, privacy: .public)" + ) + } + } + + func loadRoutines(connectionId: UUID, database: String, schema: String?) async { + guard isConnected(connectionId) else { return } + let key = Self.objectsKey(connectionId: connectionId, database: database, schema: schema) + switch routinesState[key] ?? .idle { + case .loaded, .loading: return + case .idle, .failed: break + } + routinesState[key] = .loading + let normalizedSchema = key.schema + do { + let list = try await routinesDedup.execute(key: key) { [self] in + try await MetadataConnectionPool.shared.withDriver( + connectionId: connectionId, + database: database, + schema: normalizedSchema, + workload: .bulk + ) { driver in + let procedures = try await driver.fetchProcedures(schema: normalizedSchema) + let functions = try await driver.fetchFunctions(schema: normalizedSchema) + return procedures + functions } } - objects[key] = .loaded(result) + routinesState[key] = .loaded(list) } catch is CancellationError { - if case .loading = objects[key] { objects[key] = .idle } + if case .loading = routinesState[key] { routinesState[key] = .idle } } catch { - objects[key] = .failed(error.localizedDescription) + routinesState[key] = .failed(error.localizedDescription) Self.logger.warning( - "objects load failed db=\(database, privacy: .public) schema=\(schema ?? "nil", privacy: .public) error=\(error.localizedDescription, privacy: .public)" + "routines load failed db=\(database, privacy: .public) schema=\(schema ?? "nil", privacy: .public) error=\(error.localizedDescription, privacy: .public)" ) } } @@ -171,9 +199,13 @@ final class DatabaseTreeMetadataService { func refreshObjects(connectionId: UUID, database: String, schema: String?) async { let key = Self.objectsKey(connectionId: connectionId, database: database, schema: schema) - await objectsDedup.cancel(key: key) - objects.removeValue(forKey: key) - await loadObjects(connectionId: connectionId, database: database, schema: schema) + await tablesDedup.cancel(key: key) + await routinesDedup.cancel(key: key) + tablesState.removeValue(forKey: key) + routinesState.removeValue(forKey: key) + async let tables = loadTables(connectionId: connectionId, database: database, schema: schema) + async let routines = loadRoutines(connectionId: connectionId, database: database, schema: schema) + _ = await (tables, routines) } // MARK: - Lifecycle @@ -186,20 +218,28 @@ final class DatabaseTreeMetadataService { func handleDisconnect(connectionId: UUID) async { MetadataConnectionPool.shared.closeAll(connectionId: connectionId) let schemaKeys = schemaList.keys.filter { $0.connectionId == connectionId } - let objectKeys = objects.keys.filter { $0.connectionId == connectionId } + let objectKeys = Self.connectionObjectKeys( + tableKeys: tablesState.keys, routineKeys: routinesState.keys, connectionId: connectionId + ) await databaseDedup.cancel(key: connectionId) for key in schemaKeys { await schemaDedup.cancel(key: key) } - for key in objectKeys { await objectsDedup.cancel(key: key) } + for key in objectKeys { + await tablesDedup.cancel(key: key) + await routinesDedup.cancel(key: key) + } databaseList.removeValue(forKey: connectionId) schemaList = schemaList.filter { $0.key.connectionId != connectionId } - objects = objects.filter { $0.key.connectionId != connectionId } + tablesState = tablesState.filter { $0.key.connectionId != connectionId } + routinesState = routinesState.filter { $0.key.connectionId != connectionId } } // MARK: - Private private func resetPending(connectionId: UUID) async { let schemaKeys = schemaList.keys.filter { $0.connectionId == connectionId } - let objectKeys = objects.keys.filter { $0.connectionId == connectionId } + let objectKeys = Self.connectionObjectKeys( + tableKeys: tablesState.keys, routineKeys: routinesState.keys, connectionId: connectionId + ) if isPending(databaseList[connectionId]) { await databaseDedup.cancel(key: connectionId) @@ -207,13 +247,17 @@ final class DatabaseTreeMetadataService { for key in schemaKeys where isPending(schemaList[key]) { await schemaDedup.cancel(key: key) } - for key in objectKeys where isPending(objects[key]) { - await objectsDedup.cancel(key: key) + for key in objectKeys { + if isPending(tablesState[key]) { await tablesDedup.cancel(key: key) } + if isPending(routinesState[key]) { await routinesDedup.cancel(key: key) } } if isPending(databaseList[connectionId]) { databaseList[connectionId] = .idle } for key in schemaKeys where isPending(schemaList[key]) { schemaList[key] = .idle } - for key in objectKeys where isPending(objects[key]) { objects[key] = .idle } + for key in objectKeys { + if isPending(tablesState[key]) { tablesState[key] = .idle } + if isPending(routinesState[key]) { routinesState[key] = .idle } + } } private func isPending(_ state: MetadataLoadState?) -> Bool { @@ -247,4 +291,12 @@ final class DatabaseTreeMetadataService { let normalized: String? = (schema?.isEmpty == true) ? nil : schema return ObjectsKey(connectionId: connectionId, database: database, schema: normalized) } + + static func connectionObjectKeys( + tableKeys: some Sequence, + routineKeys: some Sequence, + connectionId: UUID + ) -> [ObjectsKey] { + Array(Set(tableKeys).union(routineKeys)).filter { $0.connectionId == connectionId } + } } diff --git a/TablePro/Views/Sidebar/DatabaseTreeView.swift b/TablePro/Views/Sidebar/DatabaseTreeView.swift index d0f55f462..cc84c7c73 100644 --- a/TablePro/Views/Sidebar/DatabaseTreeView.swift +++ b/TablePro/Views/Sidebar/DatabaseTreeView.swift @@ -43,6 +43,40 @@ struct DatabaseTreeSchemaRef: Identifiable { } } +private enum SchemaTreeRow: Identifiable { + case loading + case empty + case error(String) + case schema(DatabaseTreeSchemaRef) + + var id: String { + switch self { + case .loading: return "status.loading" + case .empty: return "status.empty" + case .error: return "status.error" + case .schema(let ref): return "schema.\(ref.id)" + } + } +} + +private enum ObjectTreeRow: Identifiable { + case loading + case empty + case error(String) + case table(DatabaseTreeTableRef) + case routine(DatabaseTreeRoutineRef) + + var id: String { + switch self { + case .loading: return "status.loading" + case .empty: return "status.empty" + case .error: return "status.error" + case .table(let ref): return "table.\(ref.id)" + case .routine(let ref): return "routine.\(ref.id)" + } + } +} + struct DatabaseTreeView: View { @Bindable private var treeService = DatabaseTreeMetadataService.shared @@ -232,63 +266,117 @@ struct DatabaseTreeView: View { @ViewBuilder private func schemasContent(for database: String) -> some View { + Group { + ForEach(schemaRows(for: database)) { row in + schemaRow(row, database: database) + } + } + .task(id: "\(database)|\(connectionToken)") { + await treeService.loadSchemas(connectionId: connectionId, database: database) + } + } + + private func schemaRows(for database: String) -> [SchemaTreeRow] { switch treeService.schemaListState(connectionId: connectionId, database: database) { case .idle, .loading: - loadingRow(String(localized: "Loading schemas\u{2026}")) - .task(id: "\(database)|\(connectionToken)") { - await treeService.loadSchemas(connectionId: connectionId, database: database) - } + return [.loading] case .failed(let message): - errorRow(message) + return [.error(message)] case .loaded(let schemas): let visible = visibleSchemas(database: database, all: schemas) - if visible.isEmpty { - emptyRow(String(localized: "No schemas")) - } else { - ForEach(visible.map { DatabaseTreeSchemaRef(database: database, schema: $0) }) { ref in - DisclosureGroup(isExpanded: schemaExpansionBinding(database: ref.database, schema: ref.schema)) { - objectsContent(database: ref.database, schema: ref.schema) - } label: { - schemaHeader(database: ref.database, schema: ref.schema) - } - } + guard !visible.isEmpty else { return [.empty] } + return visible.map { .schema(DatabaseTreeSchemaRef(database: database, schema: $0)) } + } + } + + @ViewBuilder + private func schemaRow(_ row: SchemaTreeRow, database: String) -> some View { + switch row { + case .loading: + loadingRow(String(localized: "Loading schemas\u{2026}")) + case .empty: + emptyRow(String(localized: "No schemas")) + case .error(let message): + errorRow(message) + case .schema(let ref): + DisclosureGroup(isExpanded: schemaExpansionBinding(database: ref.database, schema: ref.schema)) { + objectsContent(database: ref.database, schema: ref.schema) + } label: { + schemaHeader(database: ref.database, schema: ref.schema) } } } @ViewBuilder private func objectsContent(database: String, schema: String?) -> some View { - switch treeService.objectsState(connectionId: connectionId, database: database, schema: schema) { + Group { + ForEach(objectRows(database: database, schema: schema)) { row in + objectRow(row, database: database, schema: schema) + } + } + .task(id: "tables|\(database)|\(schema ?? "")|\(connectionToken)") { + await treeService.loadTables(connectionId: connectionId, database: database, schema: schema) + } + .task(id: "routines|\(database)|\(schema ?? "")|\(connectionToken)") { + await treeService.loadRoutines(connectionId: connectionId, database: database, schema: schema) + } + } + + private func objectRows(database: String, schema: String?) -> [ObjectTreeRow] { + switch treeService.tablesLoadState(connectionId: connectionId, database: database, schema: schema) { case .idle, .loading: - loadingRow(String(localized: "Loading tables\u{2026}")) - .task(id: "\(database)|\(schema ?? "")|\(connectionToken)") { - await treeService.loadObjects(connectionId: connectionId, database: database, schema: schema) - } + return [.loading] case .failed(let message): - errorRow(message) + return [.error(message)] case .loaded: let tables = filteredTables(database: database, schema: schema) let routines = filteredRoutines(database: database, schema: schema) - if tables.isEmpty && routines.isEmpty { - emptyRow(String(localized: "No items")) - } else { - ForEach(tables.map { DatabaseTreeTableRef(database: database, schema: schema, table: $0) }) { ref in - TableRow( - table: ref.table, - isPendingTruncate: pendingTruncates.contains(ref.table.name), - isPendingDelete: pendingDeletes.contains(ref.table.name) - ) - .tag(ref) - } - ForEach(routines.map { DatabaseTreeRoutineRef(database: database, schema: schema, routine: $0) }) { ref in - RoutineRowView(routine: ref.routine) - .contextMenu { - RoutineContextMenu(routine: ref.routine) { selected in - coordinator?.showRoutineDDL(selected) - } - } + let routinesState = treeService.routinesLoadState( + connectionId: connectionId, database: database, schema: schema + ) + guard !tables.isEmpty || !routines.isEmpty else { + switch routinesState { + case .failed(let message): return [.error(message)] + case .loaded: return [.empty] + case .idle, .loading: return [.loading] } } + var rows: [ObjectTreeRow] = tables.map { + .table(DatabaseTreeTableRef(database: database, schema: schema, table: $0)) + } + rows += routines.map { + .routine(DatabaseTreeRoutineRef(database: database, schema: schema, routine: $0)) + } + if case .failed(let message) = routinesState { + rows.append(.error(message)) + } + return rows + } + } + + @ViewBuilder + private func objectRow(_ row: ObjectTreeRow, database: String, schema: String?) -> some View { + switch row { + case .loading: + loadingRow(String(localized: "Loading\u{2026}")) + case .empty: + emptyRow(String(localized: "No items")) + case .error(let message): + errorRow(message) + case .table(let ref): + TableRow( + table: ref.table, + isPendingTruncate: pendingTruncates.contains(ref.table.name), + isPendingDelete: pendingDeletes.contains(ref.table.name) + ) + .tag(ref) + case .routine(let ref): + RoutineRowView(routine: ref.routine) + .contextMenu { + RoutineContextMenu(routine: ref.routine) { selected in + coordinator?.showRoutineDDL(selected) + } + } } } diff --git a/TableProTests/Core/Services/Query/DatabaseTreeMetadataServiceTests.swift b/TableProTests/Core/Services/Query/DatabaseTreeMetadataServiceTests.swift new file mode 100644 index 000000000..853ab846d --- /dev/null +++ b/TableProTests/Core/Services/Query/DatabaseTreeMetadataServiceTests.swift @@ -0,0 +1,54 @@ +import Foundation +@testable import TablePro +import Testing + +@Suite("DatabaseTreeMetadataService") +struct DatabaseTreeMetadataServiceTests { + private typealias ObjectsKey = DatabaseTreeMetadataService.ObjectsKey + + @Test("connectionObjectKeys unions table and routine keys for the connection") + func unionsTableAndRoutineKeys() { + let connectionId = UUID() + let tableOnly = ObjectsKey(connectionId: connectionId, database: "shop", schema: "public") + let routineOnly = ObjectsKey(connectionId: connectionId, database: "shop", schema: "billing") + let shared = ObjectsKey(connectionId: connectionId, database: "shop", schema: nil) + + let keys = DatabaseTreeMetadataService.connectionObjectKeys( + tableKeys: [tableOnly, shared], + routineKeys: [routineOnly, shared], + connectionId: connectionId + ) + + #expect(Set(keys) == [tableOnly, routineOnly, shared]) + } + + @Test("connectionObjectKeys includes a routine key with no matching table key") + func includesOrphanRoutineKey() { + let connectionId = UUID() + let routineOnly = ObjectsKey(connectionId: connectionId, database: "shop", schema: "public") + + let keys = DatabaseTreeMetadataService.connectionObjectKeys( + tableKeys: [ObjectsKey](), + routineKeys: [routineOnly], + connectionId: connectionId + ) + + #expect(keys == [routineOnly]) + } + + @Test("connectionObjectKeys excludes keys from other connections") + func excludesOtherConnections() { + let connectionId = UUID() + let other = UUID() + let mine = ObjectsKey(connectionId: connectionId, database: "shop", schema: nil) + let theirs = ObjectsKey(connectionId: other, database: "shop", schema: nil) + + let keys = DatabaseTreeMetadataService.connectionObjectKeys( + tableKeys: [mine, theirs], + routineKeys: [theirs], + connectionId: connectionId + ) + + #expect(keys == [mine]) + } +} diff --git a/TableProTests/Core/Services/Query/MetadataLoadStateTests.swift b/TableProTests/Core/Services/Query/MetadataLoadStateTests.swift new file mode 100644 index 000000000..b6ae8ed5d --- /dev/null +++ b/TableProTests/Core/Services/Query/MetadataLoadStateTests.swift @@ -0,0 +1,13 @@ +@testable import TablePro +import Testing + +@Suite("MetadataLoadState") +struct MetadataLoadStateTests { + @Test("value returns the payload only for loaded") + func valueOnlyWhenLoaded() { + #expect(MetadataLoadState<[String]>.idle.value == nil) + #expect(MetadataLoadState<[String]>.loading.value == nil) + #expect(MetadataLoadState<[String]>.failed("boom").value == nil) + #expect(MetadataLoadState<[String]>.loaded(["a", "b"]).value == ["a", "b"]) + } +}