From 298e625ee124c7e9a1f88f6fb197c66cfdff729e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 19 Oct 2026 23:28:04 +0700 Subject: [PATCH 1/2] fix(datagrid): show Table Info once a table tab's rows arrive The inspector's Table Info load ran as a table tab opened, returned because the rows had not arrived, and nothing ran it again when they did, so the panel showed "No Row Selected" until the pane was switched away and back. The load now runs from one task keyed on the tab, the table's identity, whether its rows have arrived and whether the inspector is on screen. The cache is the published source the inspector snapshot follows, matched on the whole table identity. A late read is dropped only when its tab moved to another table, never replaces a newer read, and a read across a comment edit cannot mark the cache current. A comment edit keeps the old numbers until the new ones land, a rename keeps them, query tabs show none, and the filter bar's suggestions follow the tab's scope. --- .../Models/UI/RowInspectorContext.swift | 8 +- .../Views/Filter/QueryTabFilterPanel.swift | 16 +- ...ntentCoordinator+DatabaseObjectTools.swift | 8 +- ...ainContentCoordinator+RenameAdoption.swift | 2 + ...MainContentCoordinator+SchemaLoading.swift | 112 +++-- .../Extensions/MainContentView+Helpers.swift | 69 +-- .../Views/Main/MainContentCoordinator.swift | 11 +- .../TablePro/Views/Main/MainContentView.swift | 20 +- .../Autocomplete/SQLSchemaProviderTests.swift | 21 +- .../TrailingPaneRevealTests.swift | 77 +++- ...ContentCoordinatorTableMetadataTests.swift | 426 ++++++++++++++---- .../Views/Main/TableInfoTaskKeyTests.swift | 67 +++ 12 files changed, 659 insertions(+), 178 deletions(-) create mode 100644 apple/TableProTests/Views/Main/TableInfoTaskKeyTests.swift diff --git a/apple/TablePro/Models/UI/RowInspectorContext.swift b/apple/TablePro/Models/UI/RowInspectorContext.swift index 424173f095..7a1eb23bd1 100644 --- a/apple/TablePro/Models/UI/RowInspectorContext.swift +++ b/apple/TablePro/Models/UI/RowInspectorContext.swift @@ -28,10 +28,10 @@ internal struct RowInspectorContext: Equatable { internal let isEditable: Bool internal let isRowDeleted: Bool - /// Present only on a tab that actually has a table. The old context handed over - /// `coordinator.tableMetadata` unconditionally, and that slot is latest-wins and cleared only - /// on teardown, so a query tab, a dashboard or an ER diagram showed the statistics of whichever - /// table had been opened last, including after its tab was closed. + /// Present only on a tab that actually has a table. The old context handed over the + /// coordinator's one metadata slot unconditionally, latest-wins and cleared only on teardown, so + /// a query tab, a dashboard or an ER diagram showed the statistics of whichever table had been + /// opened last, including after its tab was closed. internal let tableMetadata: TableMetadata? /// The same row the fields mode shows, as raw cell values, because the JSON mode decides from diff --git a/apple/TablePro/Views/Filter/QueryTabFilterPanel.swift b/apple/TablePro/Views/Filter/QueryTabFilterPanel.swift index f00d31777c..d21db96da9 100644 --- a/apple/TablePro/Views/Filter/QueryTabFilterPanel.swift +++ b/apple/TablePro/Views/Filter/QueryTabFilterPanel.swift @@ -42,14 +42,26 @@ internal struct QueryTabFilterPanel: View { .onChange(of: columns) { _ in refreshRawSQLCompletionProvider() } - .onChange(of: coordinator.currentTableName) { _ in + .onChange(of: suggestionSource) { _ in refreshRawSQLCompletionProvider() } - .task(id: coordinator.currentTableName) { + .task(id: suggestionSource) { await loadFieldPaths() } } + /// What both kinds of suggestion are read from: the table and the tab's scope. The name alone + /// stayed equal across two tabs on `users` in two databases, so the second kept the first one's + /// field paths and completions. + private struct SuggestionSource: Hashable { + let tableName: String? + let scope: DatabaseScope? + } + + private var suggestionSource: SuggestionSource { + SuggestionSource(tableName: coordinator.currentTableName, scope: coordinator.selectedTabScope) + } + private var configuration: FilterPanelConfiguration { FilterPanelConfiguration( columns: columns, diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+DatabaseObjectTools.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+DatabaseObjectTools.swift index 8decfe249b..0ec3e8b810 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+DatabaseObjectTools.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+DatabaseObjectTools.swift @@ -139,11 +139,9 @@ extension MainContentCoordinator { let definition = TableFreshness.Change(extent: .definition, at: change.changedAt) refreshTableTabs(for: definition, excluding: change.originTabId) { showingIds.contains($0.id) } case .comment: - for tab in showing { - tableMetadataCache.removeValue(forKey: tab.id) - } - if let selected, let tableName = selected.tableContext.tableName { - Task { await loadTableMetadata(tableName: tableName, for: selected) } + markTableMetadataStale(forTabs: showingIds) + if let selected { + Task { await loadTableMetadata(for: selected) } } refreshStructure(ofTabs: showing) case .dropped: diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+RenameAdoption.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+RenameAdoption.swift index 9ef219e1d3..fa18aeeba4 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+RenameAdoption.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+RenameAdoption.swift @@ -22,6 +22,7 @@ extension MainContentCoordinator { tab.title = newName } tabManager.markTabRenamed(tabManager.tabs[index].id) + followRename(ofTableMetadataFor: tabManager.tabs[index]) /// The browse query still names the old table, so the next page, sort or filter would /// run against a name the server no longer has. rebuildTableQuery(at: index) @@ -45,6 +46,7 @@ extension MainContentCoordinator { tab.tableContext.databaseName = toDatabase if schema != nil { tab.tableContext.schemaName = toSchema } } + followRename(ofTableMetadataFor: tabManager.tabs[index]) rebuildTableQuery(at: index) } } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift index 13546d51fe..aaa23b8a1c 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift @@ -7,11 +7,17 @@ import Combine import Foundation import os -/// One tab's Table Info numbers, stamped with what they were read against. +/// One tab's Table Info numbers, stamped with the table and the execution they were read against. struct TableMetadataCacheEntry { - let tableName: String + var table: TableTabIdentity let lastExecutedAt: Date? + /// The coordinator's `tableMetadataGeneration` when the read started. + let generation: Int let metadata: TableMetadata + /// Set when the object changed without the tab running again, as a comment edit does. The + /// numbers stay on screen until a refetch replaces them, rather than the panel going empty while + /// it runs and staying empty if it fails. + var isStale = false } extension MainContentCoordinator { @@ -162,57 +168,93 @@ extension MainContentCoordinator { } } - /// Whether the Table Info panel already holds this tab's numbers. + /// The table a tab's Table Info describes, or nil on a tab with no table of its own, such as a + /// query. The load's task key, the load and the read all ask this one question, so they agree on + /// which tabs have Table Info and on when a retarget makes the cached numbers another table's. + func tableInfoIdentity(of tab: QueryTab) -> TableTabIdentity? { + tab.tableIdentity(browsing: browseDatabaseName) + } + + /// Whether the cache already holds this tab's numbers. /// /// The panel used to be backed by one latest-wins slot, so alternating between two tabs missed /// it on every switch and ran a statistics command (`collStats` on MongoDB) each time. The entry - /// is keyed by tab and stamped with the tab's last execution, so a reload, a page or a filter - /// refetches while a plain switch does not. (#2424) - func hasCurrentTableMetadata(for tab: QueryTab, tableName: String) -> Bool { - guard let entry = tableMetadataCache[tab.id] else { return false } - return entry.tableName == tableName && entry.lastExecutedAt == tab.execution.lastExecutedAt + /// is keyed by tab and stamped with the tab's last execution, so the next load after a reload, a + /// page or a filter reads the numbers again, while a plain switch does not. (#2424) + func hasCurrentTableMetadata(for tab: QueryTab) -> Bool { + guard let entry = tableMetadataCache[tab.id], !entry.isStale else { return false } + return entry.table == tableInfoIdentity(of: tab) && entry.lastExecutedAt == tab.execution.lastExecutedAt } - /// The numbers loaded for this tab, or nil. Read per tab, never from the latest-wins slot, so a - /// table of the same name in another database cannot show them; a retarget drops the entry. - /// A refresh keeps showing them until new ones arrive. + /// The numbers loaded for this tab's table, or nil. Matched on the whole identity, so a table of + /// the same name in another database or schema never shows them. A refresh keeps showing them + /// until new ones arrive. func currentTableMetadata(for tab: QueryTab) -> TableMetadata? { - guard let tableName = tab.tableContext.tableName, - let entry = tableMetadataCache[tab.id], - entry.tableName == tableName - else { return nil } + guard let entry = tableMetadataCache[tab.id], entry.table == tableInfoIdentity(of: tab) else { return nil } return entry.metadata } - func loadTableMetadata(tableName: String, for tab: QueryTab) async { - if let entry = tableMetadataCache[tab.id], - entry.tableName == tableName, - entry.lastExecutedAt == tab.execution.lastExecutedAt { - tableMetadata = entry.metadata - return - } - // Only the inspector draws these numbers; it reads them when it comes on screen. - guard trailingPaneState?.inspector.isOnScreen == true else { return } - guard let scope = selectedTabScope else { - Self.logger.error("Skipped table metadata load: no database bound to the selected tab") + /// Reads the tab's numbers once its rows have arrived, and only while the inspector, the one + /// view that draws them, is on screen. A read before the rows would be stamped with no execution + /// and read again the moment they land. + func loadTableMetadata(for tab: QueryTab) async { + guard let table = tableInfoIdentity(of: tab), + let executedAt = tab.execution.lastExecutedAt, + trailingPaneState?.inspector.isOnScreen == true, + !hasCurrentTableMetadata(for: tab) + else { return } + guard let scope = scope(for: tab) else { + Self.logger.error("Skipped table metadata load: no database bound to the tab") return } + let generation = tableMetadataGeneration do { let metadata = try await services.databaseManager.withMetadataDriver(scope: scope) { driver in - try await driver.fetchTableMetadata(tableName: tableName) + try await driver.fetchTableMetadata(tableName: table.table) + } + /// The fetch cannot be cancelled, so it can land after its tab moved onto another table on + /// the same id, or closed, and stored then it would show the numbers of what the tab used + /// to show. A tab that only ran again keeps them, stamped with the run they were read for, + /// so they show and the next load reads again; the load key does not move on a page, and + /// dropping them would leave the panel empty. A read that started later and landed first, + /// after another run or after an invalidation, is kept. + guard let live = tabManager.tabs.first(where: { $0.id == tab.id }), + tableInfoIdentity(of: live) == table + else { return } + if let existing = tableMetadataCache[tab.id], existing.table == table, + let existingStamp = existing.lastExecutedAt, + (existingStamp, existing.generation) > (executedAt, generation) { + return } tableMetadataCache[tab.id] = TableMetadataCacheEntry( - tableName: tableName, - lastExecutedAt: tab.execution.lastExecutedAt, - metadata: metadata + table: table, + lastExecutedAt: executedAt, + generation: generation, + metadata: metadata, + isStale: generation != tableMetadataGeneration ) - /// The fetch is not cancellable, so a `.task` the user has already navigated away from - /// still completes. Without this the numbers of the collection they left land in the - /// panel of the one they are looking at, under its name, and stay there. - guard tabManager.selectedTabId == tab.id else { return } - tableMetadata = metadata + } catch is CancellationError { + return } catch { Self.logger.error("Failed to load table metadata: \(error.publicLogShape, privacy: .public)") } } + + /// The object changed under the numbers without the tab running again, so the next load reads + /// them afresh while the panel goes on showing the ones it has. + func markTableMetadataStale(forTabs tabIds: some Sequence) { + tableMetadataGeneration &+= 1 + for tabId in tabIds { + tableMetadataCache[tabId]?.isStale = true + } + } + + /// A rename leaves the tab on the same table under another name, so its numbers still describe + /// it, and the panel draws none of them from the name. Moved onto the tab's new identity, they + /// stay on screen with no read at all. + func followRename(ofTableMetadataFor tab: QueryTab) { + guard var entry = tableMetadataCache[tab.id], let table = tableInfoIdentity(of: tab) else { return } + entry.table = table + tableMetadataCache[tab.id] = entry + } } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift b/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift index 2e2af76545..4ddf4a55df 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift @@ -12,29 +12,26 @@ import TableProPluginKit extension MainContentView { // MARK: - Helper Methods - /// Keyed on the tab and the table's whole identity. A name alone stayed equal across - /// `tenant_a.orders` and `tenant_b.orders`, so the second never loaded its own numbers. + /// What the Table Info load waits for, so it runs again once each of them moves. + /// + /// The load waits for the tab's rows. Without `hasRows`, a table tab that had just opened ran the + /// load before they arrived and never again, so the panel showed no numbers until the inspector + /// left the screen and came back. Whether rows arrived, not when: a page, a sort or a filter + /// runs the query again without changing the size or the row count, and a read per page would + /// count up to 100,001 rows on SQLite and run `collStats` on MongoDB each time. The whole + /// identity, not the name, because `tenant_a.orders` and `tenant_b.orders` are two tables. struct TableInfoTaskKey: Hashable { let tabId: UUID let table: TableTabIdentity? - } - - var tableInfoTaskKey: TableInfoTaskKey? { - currentTab.map { TableInfoTaskKey(tabId: $0.id, table: $0.tableIdentity(browsing: "")) } - } - - func loadTableMetadataIfNeeded() async { - guard let tab = currentTab, - let tableName = tab.tableContext.tableName, - !coordinator.hasCurrentTableMetadata(for: tab, tableName: tableName) - else { return } - await coordinator.loadTableMetadata(tableName: tableName, for: tab) - } - - func loadTableInfo() async { - guard currentTab?.execution.lastExecutedAt != nil else { return } - await loadTableMetadataIfNeeded() - scheduleInspectorUpdate() + let hasRows: Bool + let isInspectorOnScreen: Bool + + init(tab: QueryTab, table: TableTabIdentity?, isInspectorOnScreen: Bool) { + tabId = tab.id + self.table = table + hasRows = tab.execution.lastExecutedAt != nil + self.isInspectorOnScreen = isInspectorOnScreen + } } func handleConnectionStatusChange() { @@ -125,12 +122,10 @@ extension MainContentView { ) } - /// Nil on a tab that has no table of its own. - /// - /// Read from the entry cached for this tab, never from `coordinator.tableMetadata`, the single - /// latest-wins slot. Gated on the name, the slot showed a query tab, a dashboard, or a table of - /// the same name in another database the numbers of whichever table had loaded last. - private var tableMetadataForCurrentTab: TableMetadata? { + /// Nil on a tab that has no table of its own. Read from the entry cached for this tab: a single + /// latest-wins slot showed a query tab, a dashboard, or a table of the same name in another + /// database the numbers of whichever table had loaded last. + var tableMetadataForCurrentTab: TableMetadata? { currentTab.flatMap { coordinator.currentTableMetadata(for: $0) } } @@ -195,3 +190,25 @@ extension MainContentView { return lines.joined(separator: "\n") } } + +/// Runs the Table Info load where the inspector's on-screen state is observed. +/// +/// `MainContentView` does not observe the inspector, whose context changes on every selection, so a +/// key built in its body never saw `isOnScreen` move. Observed here, only this modifier redraws. +internal struct TableInfoLoadModifier: ViewModifier { + @ObservedObject var inspector: RowInspectorState + let tab: QueryTab? + let table: TableTabIdentity? + let load: (QueryTab) async -> Void + + func body(content: Content) -> some View { + content.task(id: key) { + guard let tab else { return } + await load(tab) + } + } + + private var key: MainContentView.TableInfoTaskKey? { + tab.map { MainContentView.TableInfoTaskKey(tab: $0, table: table, isInspectorOnScreen: inspector.isOnScreen) } + } +} diff --git a/apple/TablePro/Views/Main/MainContentCoordinator.swift b/apple/TablePro/Views/Main/MainContentCoordinator.swift index aa5a30d286..a5df9ef068 100644 --- a/apple/TablePro/Views/Main/MainContentCoordinator.swift +++ b/apple/TablePro/Views/Main/MainContentCoordinator.swift @@ -295,7 +295,6 @@ final class MainContentCoordinator: ObservableObject { // MARK: - Published State @Published var cursorPositions: [CursorPosition] = [] - @Published var tableMetadata: TableMetadata? @Published var activeSheet: ActiveSheet? { didSet { releaseImportFileUnlessImporting() } } @@ -316,7 +315,13 @@ final class MainContentCoordinator: ObservableObject { var displayFormatsCache: [UUID: DisplayFormatsCacheEntry] = [:] var displayOrderCache: [UUID: DisplayOrderCacheEntry] = [:] var displayStateCache: [UUID: DisplayStateCacheEntry] = [:] - var tableMetadataCache: [UUID: TableMetadataCacheEntry] = [:] + /// Published because the inspector draws from it: a load that lands, a comment refetch and a + /// rename each reach the panel because the view rebuilds its snapshot when this changes. + @Published var tableMetadataCache: [UUID: TableMetadataCacheEntry] = [:] + /// Moves on every Table Info invalidation, so a read in flight across one cannot mark the cache + /// current: its numbers may predate the change. One counter for the window rather than one per + /// tab, because an invalidation can arrive before a tab's first read has made an entry to mark. + var tableMetadataGeneration = 0 var displayStateClock = 0 let schemaColumns = SchemaColumnStore() @@ -1023,8 +1028,6 @@ final class MainContentCoordinator: ObservableObject { changeManager.clearChanges() changeManager.pluginDriver = nil - tableMetadata = nil - services.schemaProviderRegistry.release(for: connection.id) services.schemaProviderRegistry.purgeUnused() Self.lifecycleLogger.info( diff --git a/apple/TablePro/Views/Main/MainContentView.swift b/apple/TablePro/Views/Main/MainContentView.swift index f0c91f1cd8..d6fafcdaae 100644 --- a/apple/TablePro/Views/Main/MainContentView.swift +++ b/apple/TablePro/Views/Main/MainContentView.swift @@ -329,9 +329,13 @@ struct MainContentView: View { configureWindow(window) } } - .task(id: tableInfoTaskKey) { - await loadTableInfo() - } + .modifier(TableInfoLoadModifier( + inspector: trailingPaneState.inspector, + tab: currentTab, + table: currentTab.flatMap { coordinator.tableInfoIdentity(of: $0) } + ) { [coordinator] tab in + await coordinator.loadTableMetadata(for: tab) + }) /// Keyed on the connection alone. The only driver that answers `fetchSessionContexts` /// is Snowflake, which pays two round trips for it, so this must not reload per query. /// It lives with the connection's content rather than with the toolbar control it used @@ -343,6 +347,12 @@ struct MainContentView: View { .onChange(of: inspectorTrigger) { _ in scheduleInspectorUpdate() } + /// The numbers land after the rows that moved the trigger above, and a comment refetch + /// moves nothing else, so the snapshot follows the cache on its own. Only the snapshot: + /// the edit state does not depend on them, and rebuilding it would end a typed undo run. + .onChange(of: tableMetadataForCurrentTab) { _ in + updateInspectorContext() + } /// The JSON rendering draws the snapshot the context carries, and an edit made in the /// fields rendering changes the row under it without moving anything `InspectorTrigger` /// watches. Rebuilding on the switch is enough: the two renderings are never on screen @@ -350,10 +360,6 @@ struct MainContentView: View { .onValueChange(of: \.viewMode, in: trailingPaneState.inspector) { _, _ in updateInspectorContext() } - .onValueChange(of: \.isOnScreen, in: trailingPaneState.inspector) { _, isOnScreen in - guard isOnScreen else { return } - Task { await loadTableInfo() } - } /// A value window detached from a field goes on writing while the JSON rendering is the /// one on screen, and it moves nothing the trigger above watches. Debounced, because it /// commits per keystroke and rebuilding the JSON tree cancels the reader's fetches. diff --git a/apple/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift b/apple/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift index d75b10582f..cd51984f00 100644 --- a/apple/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift +++ b/apple/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift @@ -171,6 +171,8 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen var onUserQueryPaused: (@Sendable () -> Void)? private var userQueryGate: CheckedContinuation? + var isHoldingUserQuery: Bool { userQueryGate != nil } + func resumeUserQuery() { userQueryGate?.resume() userQueryGate = nil @@ -320,14 +322,31 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen var fetchTableMetadataError: Error? private(set) var fetchTableMetadataCalls: [String] = [] + /// Holds the next `fetchTableMetadata` after it has read `tableCommentToReturn`, the way a + /// statistics read is still on the wire when the tab it was started for moves on. + var pausesNextFetchTableMetadata = false + private var fetchTableMetadataGate: CheckedContinuation? + + func resumeFetchTableMetadata() { + fetchTableMetadataGate?.resume() + fetchTableMetadataGate = nil + } + func fetchTableMetadata(tableName: String) async throws -> TableMetadata { fetchTableMetadataCalls.append(tableName) if let fetchTableMetadataError { throw fetchTableMetadataError } + let comment = tableCommentToReturn + if pausesNextFetchTableMetadata { + pausesNextFetchTableMetadata = false + await withCheckedContinuation { continuation in + fetchTableMetadataGate = continuation + } + } return TableMetadata( tableName: tableName, dataSize: nil, indexSize: nil, totalSize: nil, - avgRowLength: nil, rowCount: nil, comment: tableCommentToReturn, engine: nil, + avgRowLength: nil, rowCount: nil, comment: comment, engine: nil, collation: nil, createTime: nil, updateTime: nil ) } diff --git a/apple/TableProTests/Core/Services/Infrastructure/TrailingPaneRevealTests.swift b/apple/TableProTests/Core/Services/Infrastructure/TrailingPaneRevealTests.swift index a87d1ec4d9..0b1dde861a 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/TrailingPaneRevealTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/TrailingPaneRevealTests.swift @@ -12,6 +12,7 @@ import AppKit import Foundation @testable import TablePro +import TableProTestSupport import Testing @MainActor @@ -272,6 +273,55 @@ struct TrailingPaneRevealTests { #expect(!harness.paneState.inspector.isOnScreen) } + // MARK: - Table Info + + /// The reported bug: a table tab opens before its rows arrive, the load ran at that moment and + /// never again, and Table Info appeared only once the pane was switched to the assistant and back. + /// PGlite, because a pooled metadata connection cannot open here and PGlite reads on the session. + @Test("Table Info shows once a table tab's rows arrive, with no pane switch") + func tableInfoShowsOnceTheRowsArrive() async throws { + let harness = try Harness(type: .pglite) + defer { harness.tearDown() } + try harness.requireContent() + harness.controller.showInspector() + _ = try await harness.openTableTabBeforeItsRows("orders") + + harness.driver.resumeUserQuery() + + try await waitUntil { harness.paneState.inspector.context.tableMetadata != nil } + #expect(harness.paneState.inspector.context.tableMetadata?.tableName == "orders") + #expect(harness.driver.fetchTableMetadataCalls == ["orders"]) + } + + /// The comment arm refetched, and the result reached the cache and never the panel. + @Test("An edited comment reaches Table Info") + func editedCommentReachesTableInfo() async throws { + let harness = try Harness(type: .pglite) + defer { harness.tearDown() } + try harness.requireContent() + harness.controller.showInspector() + harness.driver.tableCommentToReturn = "old" + let tab = try await harness.openTableTabBeforeItsRows("orders") + harness.driver.resumeUserQuery() + try await waitUntil { harness.paneState.inspector.context.tableMetadata?.comment == "old" } + let coordinator = try #require(harness.selected.sessionState?.coordinator) + let shown = try #require(coordinator.tabManager.tabs.first { $0.id == tab.id }) + + harness.driver.tableCommentToReturn = "new" + coordinator.applyObjectChange(DatabaseObjectChange( + connectionId: coordinator.connectionId, + scope: DatabaseScope( + connectionId: coordinator.connectionId, + database: shown.tableContext.resolvedDatabaseName(browsing: coordinator.browseDatabaseName), + schema: shown.tableContext.schemaName + ), + name: "orders", + kind: .comment + )) + + try await waitUntil { harness.paneState.inspector.context.tableMetadata?.comment == "new" } + } + // MARK: - Helpers private static func context( @@ -321,13 +371,14 @@ struct TrailingPaneRevealTests { let selected: ConnectionWorkspace let paneState: TrailingPaneState let window: NSWindow + let driver: MockDatabaseDriver private let connection: DatabaseConnection private let suite: EphemeralDefaults /// `contentMode` is set before the window is built, which is how the welcome window's Open in /// Agent Mode lands: the mode is on before the connect, so the browse content never mounts. - init(contentMode: ConnectionWorkspaceContentMode = .browse) throws { - connection = TestFixtures.makeConnection(name: "Trailing", type: .mysql) + init(type: DatabaseType = .mysql, contentMode: ConnectionWorkspaceContentMode = .browse) throws { + connection = TestFixtures.makeConnection(name: "Trailing", type: type) suite = try EphemeralDefaults() let defaults = suite.defaults let registryDirectory = FileManager.default.temporaryDirectory @@ -360,7 +411,8 @@ struct TrailingPaneRevealTests { window.contentViewController = controller window.orderFront(nil) - var session = ConnectionSession(connection: connection, driver: MockDatabaseDriver(connection: connection)) + driver = MockDatabaseDriver(connection: connection) + var session = ConnectionSession(connection: connection, driver: driver) session.status = .connected DatabaseManager.shared.injectSession(session, for: connection.id) controller.refreshFromActiveSessions() @@ -374,6 +426,25 @@ struct TrailingPaneRevealTests { try #require(controller.currentPane == .content, "The connection has no content behind it") } + /// A table tab the way the sidebar opens one: selected and drawn by the window, its row + /// query held on the wire. Returns once the query is held and the inspector names the tab, + /// by which point every task keyed on the tab has started. `resumeUserQuery()` lets the + /// rows land through the real execution path. + func openTableTabBeforeItsRows(_ tableName: String) async throws -> QueryTab { + let tabManager = try #require(selected.sessionState?.tabManager) + driver.pausesNextUserQuery = true + let tab = QueryTab( + title: tableName, + query: "SELECT * FROM \(tableName)", + tabType: .table, + tableName: tableName + ) + tabManager.adoptTab(tab) + try await waitUntil { driver.isHoldingUserQuery } + try await waitUntil { paneState.inspector.context.subject.title?.hasSuffix(tableName) == true } + return tab + } + static func menuItem(_ action: Selector) -> NSMenuItem { NSMenuItem(title: "", action: action, keyEquivalent: "") } diff --git a/apple/TableProTests/Views/Main/MainContentCoordinatorTableMetadataTests.swift b/apple/TableProTests/Views/Main/MainContentCoordinatorTableMetadataTests.swift index 11cb525d7f..5c9680ba30 100644 --- a/apple/TableProTests/Views/Main/MainContentCoordinatorTableMetadataTests.swift +++ b/apple/TableProTests/Views/Main/MainContentCoordinatorTableMetadataTests.swift @@ -4,10 +4,12 @@ // import AppKit +import Combine import Foundation import SwiftUI import TableProEditorKit import TableProPluginKit +import TableProTestSupport import Testing @testable import TablePro @@ -25,20 +27,28 @@ struct MainContentCoordinatorTableMetadataTests { return (coordinator, tabManager) } - private func addTableTab(to tabManager: QueryTabManager, tableName: String) -> QueryTab { + /// Stamped by default, the way a tab looks once its rows have arrived. The bug this suite guards + /// lives in the moment before that, so the cases about it pass `executedAt: nil`. + private func addTableTab( + to tabManager: QueryTabManager, + tableName: String, + database: String = "", + executedAt: Date? = Date() + ) -> QueryTab { var tab = QueryTab( title: tableName, query: "SELECT * FROM \(tableName)", tabType: .table, tableName: tableName ) - tab.execution.lastExecutedAt = Date() + tab.tableContext.databaseName = database + tab.execution.lastExecutedAt = executedAt tabManager.tabs.append(tab) tabManager.selectedTabId = tab.id return tab } - private func metadata(_ tableName: String, rowCount: Int64) -> TableMetadata { + private func metadata(_ tableName: String, rowCount: Int64 = 0, comment: String? = nil) -> TableMetadata { TableMetadata( tableName: tableName, dataSize: nil, @@ -46,7 +56,7 @@ struct MainContentCoordinatorTableMetadataTests { totalSize: nil, avgRowLength: nil, rowCount: rowCount, - comment: nil, + comment: comment, engine: nil, collation: nil, createTime: nil, @@ -54,109 +64,140 @@ struct MainContentCoordinatorTableMetadataTests { ) } + private func cache(_ metadata: TableMetadata, for tab: QueryTab, in coordinator: MainContentCoordinator) throws { + let table = try #require(coordinator.tableInfoIdentity(of: tab)) + coordinator.tableMetadataCache[tab.id] = TableMetadataCacheEntry( + table: table, + lastExecutedAt: tab.execution.lastExecutedAt, + generation: coordinator.tableMetadataGeneration, + metadata: metadata + ) + } + + private func liveTab(_ id: UUID, in tabManager: QueryTabManager) throws -> QueryTab { + try #require(tabManager.tabs.first { $0.id == id }) + } + + // MARK: - The cache + @Test("Alternating between two tables fetches once per table, not once per switch") - func alternatingTabsHitTheCache() { + func alternatingTabsHitTheCache() throws { let (coordinator, tabManager) = makeCoordinator() let orders = addTableTab(to: tabManager, tableName: "orders") let customers = addTableTab(to: tabManager, tableName: "customers") - #expect(!coordinator.hasCurrentTableMetadata(for: orders, tableName: "orders")) - coordinator.tableMetadataCache[orders.id] = TableMetadataCacheEntry( - tableName: "orders", - lastExecutedAt: orders.execution.lastExecutedAt, - metadata: metadata("orders", rowCount: 10) - ) - coordinator.tableMetadataCache[customers.id] = TableMetadataCacheEntry( - tableName: "customers", - lastExecutedAt: customers.execution.lastExecutedAt, - metadata: metadata("customers", rowCount: 20) - ) + #expect(!coordinator.hasCurrentTableMetadata(for: orders)) + try cache(metadata("orders", rowCount: 10), for: orders, in: coordinator) + try cache(metadata("customers", rowCount: 20), for: customers, in: coordinator) - #expect(coordinator.hasCurrentTableMetadata(for: orders, tableName: "orders")) - #expect(coordinator.hasCurrentTableMetadata(for: customers, tableName: "customers")) + #expect(coordinator.hasCurrentTableMetadata(for: orders)) + #expect(coordinator.hasCurrentTableMetadata(for: customers)) } @Test("A table of the same name in another database never shows the other one's numbers") - func sameNameInAnotherDatabaseIsNotShown() { + func sameNameInAnotherDatabaseIsNotShown() throws { let (coordinator, tabManager) = makeCoordinator() - var tenantA = addTableTab(to: tabManager, tableName: "orders") - tenantA.tableContext.databaseName = "tenant_a" - tabManager.updateTab(tenantA) - var tenantB = addTableTab(to: tabManager, tableName: "orders") - tenantB.tableContext.databaseName = "tenant_b" - tabManager.updateTab(tenantB) - + let tenantA = addTableTab(to: tabManager, tableName: "orders", database: "tenant_a") + let tenantB = addTableTab(to: tabManager, tableName: "orders", database: "tenant_b") let tenantAMetadata = metadata("orders", rowCount: 10) - coordinator.tableMetadataCache[tenantA.id] = TableMetadataCacheEntry( - tableName: "orders", - lastExecutedAt: tenantA.execution.lastExecutedAt, - metadata: tenantAMetadata - ) - coordinator.tableMetadata = tenantAMetadata + try cache(tenantAMetadata, for: tenantA, in: coordinator) + + var retargeted = tenantA + retargeted.tableContext.databaseName = "tenant_b" #expect(coordinator.currentTableMetadata(for: tenantA) == tenantAMetadata) #expect(coordinator.currentTableMetadata(for: tenantB) == nil) + #expect(coordinator.currentTableMetadata(for: retargeted) == nil) } /// A refresh never clears what it refreshes: the panel keeps the last numbers until new ones - /// arrive, while the cache still reports them stale so the next load refetches. - @Test("Re-running the tab's query keeps its numbers on screen") - func reExecutingKeepsTheShownNumbers() { + /// arrive, while the cache reports them stale so the next load refetches. + @Test("Re-running the tab's query keeps its numbers on screen and makes them stale") + func reExecutingKeepsTheShownNumbers() throws { let (coordinator, tabManager) = makeCoordinator() var orders = addTableTab(to: tabManager, tableName: "orders") let shown = metadata("orders", rowCount: 10) - coordinator.tableMetadataCache[orders.id] = TableMetadataCacheEntry( - tableName: "orders", - lastExecutedAt: orders.execution.lastExecutedAt, - metadata: shown - ) + try cache(shown, for: orders, in: coordinator) orders.execution.lastExecutedAt = Date(timeIntervalSince1970: 1) #expect(coordinator.currentTableMetadata(for: orders) == shown) - #expect(!coordinator.hasCurrentTableMetadata(for: orders, tableName: "orders")) + #expect(!coordinator.hasCurrentTableMetadata(for: orders)) } - @Test("Re-running the tab's query makes its cached numbers stale") - func reExecutingInvalidatesTheEntry() { + @Test("Retargeting a tab to another table hides and invalidates its cached numbers") + func retargetingInvalidatesTheEntry() throws { let (coordinator, tabManager) = makeCoordinator() - var orders = addTableTab(to: tabManager, tableName: "orders") - coordinator.tableMetadataCache[orders.id] = TableMetadataCacheEntry( - tableName: "orders", - lastExecutedAt: orders.execution.lastExecutedAt, - metadata: metadata("orders", rowCount: 10) - ) + let orders = addTableTab(to: tabManager, tableName: "orders") + try cache(metadata("orders", rowCount: 10), for: orders, in: coordinator) - orders.execution.lastExecutedAt = Date(timeIntervalSince1970: 1) - #expect(!coordinator.hasCurrentTableMetadata(for: orders, tableName: "orders")) + var retargeted = orders + retargeted.tableContext.tableName = "customers" + + #expect(!coordinator.hasCurrentTableMetadata(for: retargeted)) + #expect(coordinator.currentTableMetadata(for: retargeted) == nil) } - @Test("Retargeting a tab to another table makes its cached numbers stale") - func retargetingInvalidatesTheEntry() { + /// The docs say a tab with no table of its own, such as a query, leaves Table Info empty. A + /// query tab can still carry the name of the table its result came from. + @Test("A query tab has no Table Info, whatever table its result came from") + func queryTabHasNoTableInfo() { + let (coordinator, tabManager) = makeCoordinator() + var query = QueryTab(title: "Query", query: "SELECT * FROM orders", tabType: .query, tableName: "orders") + query.execution.lastExecutedAt = Date() + tabManager.tabs.append(query) + + #expect(coordinator.tableInfoIdentity(of: query) == nil) + #expect(coordinator.currentTableMetadata(for: query) == nil) + } + + @Test("Closed tabs take their cached numbers with them") + func closingATabPrunesTheEntry() throws { let (coordinator, tabManager) = makeCoordinator() let orders = addTableTab(to: tabManager, tableName: "orders") - coordinator.tableMetadataCache[orders.id] = TableMetadataCacheEntry( - tableName: "orders", - lastExecutedAt: orders.execution.lastExecutedAt, - metadata: metadata("orders", rowCount: 10) - ) + let customers = addTableTab(to: tabManager, tableName: "customers") + try cache(metadata("orders", rowCount: 10), for: orders, in: coordinator) + try cache(metadata("customers", rowCount: 20), for: customers, in: coordinator) - #expect(!coordinator.hasCurrentTableMetadata(for: orders, tableName: "customers")) + coordinator.cleanupTabCaches(openTabIds: [customers.id]) + #expect(coordinator.tableMetadataCache[orders.id] == nil) + #expect(coordinator.tableMetadataCache[customers.id] != nil) } - @Test("A cached entry paints the panel without reaching the database") - func cachedEntryPaintsThePanel() async { + /// The inspector rebuilds its snapshot from a trigger that reads the cache, so a write the + /// coordinator does not announce never reaches the panel. A comment refetch did exactly that. + @Test("A write to the cache is announced to the views that observe the coordinator") + func cacheWritesArePublished() throws { let (coordinator, tabManager) = makeCoordinator() let orders = addTableTab(to: tabManager, tableName: "orders") - coordinator.tableMetadataCache[orders.id] = TableMetadataCacheEntry( - tableName: "orders", - lastExecutedAt: orders.execution.lastExecutedAt, - metadata: metadata("orders", rowCount: 42) - ) + var announcements = 0 + let subscription = coordinator.$tableMetadataCache.dropFirst().sink { _ in announcements += 1 } + defer { subscription.cancel() } - await coordinator.loadTableMetadata(tableName: "orders", for: orders) - #expect(coordinator.tableMetadata?.tableName == "orders") - #expect(coordinator.tableMetadata?.rowCount == 42) + try cache(metadata("orders"), for: orders, in: coordinator) + + #expect(announcements == 1) + } + + // MARK: - Loading + + @Test("Table Info waits for the tab's rows, then reads once they arrive") + func loadWaitsForTheRows() async throws { + let session = LiveSession() + defer { session.tearDown() } + let opened = addTableTab(to: session.tabManager, tableName: "orders", executedAt: nil) + session.paneState.inspector.setOnScreen(true) + + await session.coordinator.loadTableMetadata(for: opened) + #expect(session.driver.fetchTableMetadataCalls.isEmpty) + + var loaded = opened + loaded.execution.lastExecutedAt = Date() + session.tabManager.updateTab(loaded) + await session.coordinator.loadTableMetadata(for: loaded) + + #expect(session.driver.fetchTableMetadataCalls == ["orders"]) + #expect(session.coordinator.currentTableMetadata(for: loaded)?.tableName == "orders") } @Test("Table Info reads nothing while the inspector is off screen") @@ -165,44 +206,247 @@ struct MainContentCoordinatorTableMetadataTests { defer { session.tearDown() } let orders = addTableTab(to: session.tabManager, tableName: "orders") - await session.coordinator.loadTableMetadata(tableName: "orders", for: orders) + await session.coordinator.loadTableMetadata(for: orders) + + #expect(session.driver.fetchTableMetadataCalls.isEmpty) + #expect(session.coordinator.currentTableMetadata(for: orders) == nil) + } + + @Test("A query tab never reads Table Info") + func queryTabReadsNothing() async { + let session = LiveSession() + defer { session.tearDown() } + var query = QueryTab(title: "Query", query: "SELECT * FROM orders", tabType: .query, tableName: "orders") + query.execution.lastExecutedAt = Date() + session.tabManager.tabs.append(query) + session.tabManager.selectedTabId = query.id + session.paneState.inspector.setOnScreen(true) + + await session.coordinator.loadTableMetadata(for: query) #expect(session.driver.fetchTableMetadataCalls.isEmpty) - #expect(session.coordinator.tableMetadata == nil) } - @Test("Table Info reads the selected table once the inspector is on screen") - func onScreenInspectorReadsTheSelectedTable() async { + @Test("A cached entry answers without reaching the database") + func cachedEntryAnswersWithoutAFetch() async throws { let session = LiveSession() defer { session.tearDown() } let orders = addTableTab(to: session.tabManager, tableName: "orders") session.paneState.inspector.setOnScreen(true) + let cached = metadata("orders", rowCount: 42) + try cache(cached, for: orders, in: session.coordinator) - await session.coordinator.loadTableMetadata(tableName: "orders", for: orders) + await session.coordinator.loadTableMetadata(for: orders) - #expect(session.driver.fetchTableMetadataCalls == ["orders"]) - #expect(session.coordinator.tableMetadata?.tableName == "orders") + #expect(session.driver.fetchTableMetadataCalls.isEmpty) + #expect(session.coordinator.currentTableMetadata(for: orders) == cached) } - @Test("Closed tabs take their cached numbers with them") - func closingATabPrunesTheEntry() { - let (coordinator, tabManager) = makeCoordinator() - let orders = addTableTab(to: tabManager, tableName: "orders") - let customers = addTableTab(to: tabManager, tableName: "customers") - coordinator.tableMetadataCache[orders.id] = TableMetadataCacheEntry( + /// The read cannot be cancelled. Landing after the preview tab moved onto the same name in + /// another schema, it used to be stored and shown under the table the tab moved to. + @Test("A read that lands after its tab moved to another table is dropped") + func lateReadAfterARetargetIsDropped() async throws { + let session = LiveSession() + defer { session.tearDown() } + let sales = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + session.driver.pausesNextFetchTableMetadata = true + + let read = Task { await session.coordinator.loadTableMetadata(for: sales) } + try await waitUntil { session.driver.fetchTableMetadataCalls.count == 1 } + let retargeted = try session.tabManager.replaceTabContent( tableName: "orders", - lastExecutedAt: orders.execution.lastExecutedAt, - metadata: metadata("orders", rowCount: 10) + databaseType: session.connection.type, + schemaName: "archive" ) - coordinator.tableMetadataCache[customers.id] = TableMetadataCacheEntry( - tableName: "customers", - lastExecutedAt: customers.execution.lastExecutedAt, - metadata: metadata("customers", rowCount: 20) + try #require(retargeted) + var archive = try liveTab(sales.id, in: session.tabManager) + archive.execution.lastExecutedAt = Date(timeIntervalSinceNow: 60) + session.tabManager.updateTab(archive) + session.driver.resumeFetchTableMetadata() + await read.value + + #expect(session.coordinator.tableMetadataCache[sales.id] == nil) + #expect(session.coordinator.currentTableMetadata(for: archive) == nil) + } + + /// The same table under another identity on the same id, with no run in between, as a schema + /// that resolves after the read started. The identity alone has to drop it. + @Test("A read that lands after its tab's identity changed is dropped, whatever the stamp") + func lateReadAfterAnIdentityChangeIsDropped() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + session.driver.pausesNextFetchTableMetadata = true + + let read = Task { await session.coordinator.loadTableMetadata(for: orders) } + try await waitUntil { session.driver.fetchTableMetadataCalls.count == 1 } + var moved = try liveTab(orders.id, in: session.tabManager) + moved.tableContext.schemaName = "archive" + session.tabManager.updateTab(moved) + session.driver.resumeFetchTableMetadata() + await read.value + + #expect(session.coordinator.tableMetadataCache[orders.id] == nil) + } + + /// The load key does not move on a page, so a read dropped here would never run again and the + /// panel would stay empty: the reported bug by another route. + @Test("A read that lands after its tab ran again is kept, stamped with the run it was read for") + func lateReadAfterARerunIsKept() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + session.driver.pausesNextFetchTableMetadata = true + + let read = Task { await session.coordinator.loadTableMetadata(for: orders) } + try await waitUntil { session.driver.fetchTableMetadataCalls.count == 1 } + var paged = try liveTab(orders.id, in: session.tabManager) + paged.execution.lastExecutedAt = Date(timeIntervalSinceNow: 60) + session.tabManager.updateTab(paged) + session.driver.resumeFetchTableMetadata() + await read.value + + #expect(session.coordinator.currentTableMetadata(for: paged)?.tableName == "orders") + #expect(session.coordinator.tableMetadataCache[orders.id]?.lastExecutedAt == orders.execution.lastExecutedAt) + #expect(!session.coordinator.hasCurrentTableMetadata(for: paged)) + } + + @Test("A slow read never replaces a newer one that landed first") + func slowReadKeepsTheNewerEntry() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + session.driver.tableCommentToReturn = "old" + session.driver.pausesNextFetchTableMetadata = true + + let slow = Task { await session.coordinator.loadTableMetadata(for: orders) } + try await waitUntil { session.driver.fetchTableMetadataCalls.count == 1 } + var reloaded = try liveTab(orders.id, in: session.tabManager) + reloaded.execution.lastExecutedAt = Date(timeIntervalSinceNow: 60) + session.tabManager.updateTab(reloaded) + try cache(metadata("orders", comment: "new"), for: reloaded, in: session.coordinator) + session.driver.resumeFetchTableMetadata() + await slow.value + + #expect(session.coordinator.currentTableMetadata(for: reloaded)?.comment == "new") + #expect(session.coordinator.hasCurrentTableMetadata(for: reloaded)) + } + + // MARK: - Changes that keep the tab on the same table + + /// The comment arm used to drop the entry before its refetch, then never pushed the result. + @Test("Editing the comment keeps the old numbers on screen until the new ones land") + func commentEditKeepsTheNumbersUntilTheRefetchLands() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders", database: session.connection.database) + session.paneState.inspector.setOnScreen(true) + try cache(metadata("orders", comment: "old"), for: orders, in: session.coordinator) + session.driver.tableCommentToReturn = "new" + let scope = DatabaseScope(connectionId: session.connection.id, database: session.connection.database, schema: nil) + + session.coordinator.applyObjectChange( + DatabaseObjectChange(connectionId: session.connection.id, scope: scope, name: "orders", kind: .comment) ) - coordinator.cleanupTabCaches(openTabIds: [customers.id]) - #expect(coordinator.tableMetadataCache[orders.id] == nil) - #expect(coordinator.tableMetadataCache[customers.id] != nil) + #expect(session.coordinator.currentTableMetadata(for: orders)?.comment == "old") + #expect(!session.coordinator.hasCurrentTableMetadata(for: orders)) + try await waitUntil { session.coordinator.currentTableMetadata(for: orders)?.comment == "new" } + #expect(session.coordinator.hasCurrentTableMetadata(for: orders)) + } + + /// Editing a comment does not run the tab again, so the stamp cannot tell a read made before + /// the edit from one made after it. Stored as current, the old comment made the refetch skip. + @Test("A read in flight across a comment edit shows, and never marks the numbers current") + func readAcrossACommentEditStaysStale() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + session.driver.tableCommentToReturn = "old" + session.driver.pausesNextFetchTableMetadata = true + + let early = Task { await session.coordinator.loadTableMetadata(for: orders) } + try await waitUntil { session.driver.fetchTableMetadataCalls.count == 1 } + session.driver.tableCommentToReturn = "new" + session.coordinator.markTableMetadataStale(forTabs: [orders.id]) + session.driver.resumeFetchTableMetadata() + await early.value + + #expect(session.coordinator.currentTableMetadata(for: orders)?.comment == "old") + #expect(!session.coordinator.hasCurrentTableMetadata(for: orders)) + await session.coordinator.loadTableMetadata(for: orders) + #expect(session.coordinator.currentTableMetadata(for: orders)?.comment == "new") + #expect(session.coordinator.hasCurrentTableMetadata(for: orders)) + } + + @Test("A read started before a comment edit never replaces the one made after it") + func readBeforeACommentEditKeepsTheLaterOne() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + session.driver.tableCommentToReturn = "old" + session.driver.pausesNextFetchTableMetadata = true + + let early = Task { await session.coordinator.loadTableMetadata(for: orders) } + try await waitUntil { session.driver.fetchTableMetadataCalls.count == 1 } + session.coordinator.markTableMetadataStale(forTabs: [orders.id]) + try cache(metadata("orders", comment: "new"), for: orders, in: session.coordinator) + session.driver.resumeFetchTableMetadata() + await early.value + + #expect(session.coordinator.currentTableMetadata(for: orders)?.comment == "new") + #expect(session.coordinator.hasCurrentTableMetadata(for: orders)) + } + + @Test("Numbers marked stale stay on screen when their refetch fails") + func staleNumbersSurviveAFailedRefetch() async throws { + let session = LiveSession() + defer { session.tearDown() } + let orders = addTableTab(to: session.tabManager, tableName: "orders") + session.paneState.inspector.setOnScreen(true) + try cache(metadata("orders", comment: "old"), for: orders, in: session.coordinator) + session.driver.fetchTableMetadataError = DatabaseError.notConnected + + session.coordinator.markTableMetadataStale(forTabs: [orders.id]) + await session.coordinator.loadTableMetadata(for: orders) + + #expect(session.driver.fetchTableMetadataCalls == ["orders"]) + #expect(session.coordinator.currentTableMetadata(for: orders)?.comment == "old") + } + + @Test("Renaming the table keeps its numbers under the new name, with no read") + func renameKeepsTheNumbers() throws { + let (coordinator, tabManager) = makeCoordinator() + let orders = addTableTab(to: tabManager, tableName: "orders", database: "shop") + let shown = metadata("orders", rowCount: 10) + try cache(shown, for: orders, in: coordinator) + + coordinator.retitleTabs(ids: [orders.id], to: "orders_old") + let renamed = try liveTab(orders.id, in: tabManager) + + #expect(renamed.tableContext.tableName == "orders_old") + #expect(coordinator.currentTableMetadata(for: renamed) == shown) + #expect(coordinator.hasCurrentTableMetadata(for: renamed)) + } + + @Test("Renaming the database keeps the numbers of the tables in it") + func containerRenameKeepsTheNumbers() throws { + let (coordinator, tabManager) = makeCoordinator() + let orders = addTableTab(to: tabManager, tableName: "orders", database: "shop") + let shown = metadata("orders", rowCount: 10) + try cache(shown, for: orders, in: coordinator) + + coordinator.retargetTabs(database: "shop", schema: nil, toDatabase: "store", toSchema: nil) + let moved = try liveTab(orders.id, in: tabManager) + + #expect(moved.tableContext.databaseName == "store") + #expect(coordinator.currentTableMetadata(for: moved) == shown) } } diff --git a/apple/TableProTests/Views/Main/TableInfoTaskKeyTests.swift b/apple/TableProTests/Views/Main/TableInfoTaskKeyTests.swift new file mode 100644 index 0000000000..a28b08b98f --- /dev/null +++ b/apple/TableProTests/Views/Main/TableInfoTaskKeyTests.swift @@ -0,0 +1,67 @@ +// +// TableInfoTaskKeyTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +/// The Table Info load runs again only when this key changes, and it waits for the tab's rows. A key +/// that did not move when the rows arrived left a table tab that had just opened with no numbers +/// until the inspector left the screen and came back. +@MainActor +struct TableInfoTaskKeyTests { + private let table = TableTabIdentity(table: "orders", database: "shop", schema: nil) + + private func tableTab(executedAt: Date?) -> QueryTab { + var tab = QueryTab(title: "orders", query: "SELECT * FROM orders", tabType: .table, tableName: "orders") + tab.execution.lastExecutedAt = executedAt + return tab + } + + private func key(_ tab: QueryTab, onScreen: Bool = true) -> MainContentView.TableInfoTaskKey { + MainContentView.TableInfoTaskKey(tab: tab, table: table, isInspectorOnScreen: onScreen) + } + + @Test("The load runs again once a table tab's rows arrive") + func rowsArrivingMoveTheKey() { + var tab = tableTab(executedAt: nil) + let opened = key(tab) + + tab.execution.lastExecutedAt = Date() + + #expect(key(tab) != opened) + } + + /// A page, a sort or a filter runs the query again and changes none of the numbers. A read per + /// run would count up to 100,001 rows on SQLite and run `collStats` on MongoDB on every page. + @Test("A page, a sort or a filter does not read the statistics again") + func rerunKeepsTheKey() { + var tab = tableTab(executedAt: Date(timeIntervalSince1970: 1)) + let first = key(tab) + + tab.execution.lastExecutedAt = Date(timeIntervalSince1970: 2) + + #expect(key(tab) == first) + } + + @Test("The inspector coming on screen runs the load again") + func comingOnScreenMovesTheKey() { + let tab = tableTab(executedAt: Date()) + + #expect(key(tab, onScreen: true) != key(tab, onScreen: false)) + } + + @Test("A redraw that moves nothing the load reads keeps the key, so nothing reloads") + func unrelatedChangeKeepsTheKey() { + var tab = tableTab(executedAt: Date()) + let before = key(tab) + + tab.selectedRowIndices = [3] + tab.schemaVersion += 1 + + #expect(key(tab) == before) + } +} From 1961369b119ba854306f0bc935479da78fbae153 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 19 Oct 2026 23:29:00 +0700 Subject: [PATCH 2/2] chore(changelog): add the Table Info fragments --- apple/changelog.d/3595.fixed.1.md | 1 + apple/changelog.d/3595.fixed.2.md | 1 + apple/changelog.d/3595.fixed.3.md | 1 + apple/changelog.d/3595.fixed.md | 1 + 4 files changed, 4 insertions(+) create mode 100644 apple/changelog.d/3595.fixed.1.md create mode 100644 apple/changelog.d/3595.fixed.2.md create mode 100644 apple/changelog.d/3595.fixed.3.md create mode 100644 apple/changelog.d/3595.fixed.md diff --git a/apple/changelog.d/3595.fixed.1.md b/apple/changelog.d/3595.fixed.1.md new file mode 100644 index 0000000000..1fd3d83e59 --- /dev/null +++ b/apple/changelog.d/3595.fixed.1.md @@ -0,0 +1 @@ +Table Info keeping the old comment after Edit Comment. diff --git a/apple/changelog.d/3595.fixed.2.md b/apple/changelog.d/3595.fixed.2.md new file mode 100644 index 0000000000..970ad090e5 --- /dev/null +++ b/apple/changelog.d/3595.fixed.2.md @@ -0,0 +1 @@ +Table Info showing a same-named table's numbers from another schema, or going empty after a rename. diff --git a/apple/changelog.d/3595.fixed.3.md b/apple/changelog.d/3595.fixed.3.md new file mode 100644 index 0000000000..d9aa19a07e --- /dev/null +++ b/apple/changelog.d/3595.fixed.3.md @@ -0,0 +1 @@ +Filter bar suggesting the fields of a same-named collection in another database. diff --git a/apple/changelog.d/3595.fixed.md b/apple/changelog.d/3595.fixed.md new file mode 100644 index 0000000000..a95d3a2ee4 --- /dev/null +++ b/apple/changelog.d/3595.fixed.md @@ -0,0 +1 @@ +Table Info missing after opening a table, until the inspector was switched to the assistant and back.