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) + } +} 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.