diff --git a/CHANGELOG.md b/CHANGELOG.md index ab26208b9..e48f27cc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - iCloud Sync between the iPhone and Mac apps: the iOS app now uses the Production CloudKit environment, so a development build no longer syncs into a separate database the Mac never reads. - Exports no longer fail mid-table on servers that enforce a statement time limit; the export session disables the limit and restores it afterwards, the same way mysqldump does. (#1633) +- Refreshing a table now reloads its data even when the previous load is still running; before, the refresh was silently dropped and the grid kept stale rows. (#1637) ### Security diff --git a/TablePro/Views/Main/Child/DataTabGridDelegate.swift b/TablePro/Views/Main/Child/DataTabGridDelegate.swift index c51a90d7d..6b373f4df 100644 --- a/TablePro/Views/Main/Child/DataTabGridDelegate.swift +++ b/TablePro/Views/Main/Child/DataTabGridDelegate.swift @@ -20,7 +20,6 @@ final class DataTabGridDelegate: DataGridViewDelegate { var onAddRow: (() -> Void)? var onUndoInsert: ((Int) -> Void)? var onFilterColumn: ((String) -> Void)? - var onRefresh: (() -> Void)? // MARK: - DataGridViewDelegate @@ -44,10 +43,6 @@ final class DataTabGridDelegate: DataGridViewDelegate { onFilterColumn?(columnName) } - func dataGridRefresh() { - onRefresh?() - } - func dataGridDeleteRows(_ indices: Set) { coordinator?.deleteSelectedRows(indices: indices) } diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 17ba1aa40..71e5a6860 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -44,7 +44,6 @@ struct MainEditorContentView: View { let onFilterColumn: (String) -> Void let onApplyFilters: ([TableFilter]) -> Void let onClearFilters: () -> Void - let onRefresh: () -> Void // Pagination callbacks let onFirstPage: () -> Void @@ -182,7 +181,6 @@ struct MainEditorContentView: View { dataTabDelegate.onSortStateChanged = onSortStateChanged dataTabDelegate.onUndoInsert = onUndoInsert dataTabDelegate.onFilterColumn = onFilterColumn - dataTabDelegate.onRefresh = onRefresh } private func refreshDataTabDelegateMutableRefs() { diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift index 4abdbcd70..343f0e485 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swift @@ -33,7 +33,7 @@ extension MainContentCoordinator { // Query tabs should not auto-execute on refresh (use Cmd+Enter to execute) if let (tab, tabIndex) = tabManager.selectedTabAndIndex, tab.tabType == .table { - currentQueryTask?.cancel() + cancelCurrentQuery() rebuildTableQuery(at: tabIndex) runQuery() } @@ -42,7 +42,7 @@ extension MainContentCoordinator { } else { if let (tab, tabIndex) = tabManager.selectedTabAndIndex, tab.tabType == .table { - currentQueryTask?.cancel() + cancelCurrentQuery() rebuildTableQuery(at: tabIndex) runQuery() } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 56b70220c..f470dbb64 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -526,13 +526,24 @@ final class MainContentCoordinator { } func refreshTables() async { - guard let driver = services.databaseManager.driver(for: connectionId) else { return } schemaColumns.removeAll() - await services.schemaService.reload( - connectionId: connectionId, - driver: driver, - connection: connection - ) + let schemaService = services.schemaService + let connectionId = connectionId + let connection = connection + do { + try await services.databaseManager.withMetadataDriver( + connectionId: connectionId, + workload: .bulk + ) { driver in + await schemaService.reload( + connectionId: connectionId, + driver: driver, + connection: connection + ) + } + } catch { + Self.logger.warning("Schema refresh failed: \(error.localizedDescription, privacy: .public)") + } await reconcilePostSchemaLoad() } diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index d57475024..d3bc34b63 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -470,9 +470,6 @@ struct MainContentView: View { onClearFilters: { coordinator.clearFiltersAndReload() }, - onRefresh: { - coordinator.runQuery() - }, onFirstPage: { coordinator.goToFirstPage() }, diff --git a/TablePro/Views/Results/DataGridViewDelegate.swift b/TablePro/Views/Results/DataGridViewDelegate.swift index 4ea326f5d..536d653f3 100644 --- a/TablePro/Views/Results/DataGridViewDelegate.swift +++ b/TablePro/Views/Results/DataGridViewDelegate.swift @@ -27,7 +27,6 @@ protocol DataGridViewDelegate: AnyObject { func dataGridCanClearResults() -> Bool func dataGridHideColumn(_ columnName: String) func dataGridShowAllColumns() - func dataGridRefresh() func dataGridVisualState(forRow row: Int) -> RowVisualState? func dataGridRowView(for tableView: NSTableView, row: Int, coordinator: TableViewCoordinator) -> NSTableRowView? func dataGridEmptySpaceMenu() -> NSMenu? @@ -56,7 +55,6 @@ extension DataGridViewDelegate { func dataGridCanClearResults() -> Bool { false } func dataGridHideColumn(_ columnName: String) {} func dataGridShowAllColumns() {} - func dataGridRefresh() {} func dataGridVisualState(forRow row: Int) -> RowVisualState? { nil } func dataGridRowView(for tableView: NSTableView, row: Int, coordinator: TableViewCoordinator) -> NSTableRowView? { nil } func dataGridEmptySpaceMenu() -> NSMenu? { nil } diff --git a/TableProTests/Views/Main/MainContentCoordinatorRefreshTests.swift b/TableProTests/Views/Main/MainContentCoordinatorRefreshTests.swift new file mode 100644 index 000000000..d287bb5f5 --- /dev/null +++ b/TableProTests/Views/Main/MainContentCoordinatorRefreshTests.swift @@ -0,0 +1,166 @@ +// +// MainContentCoordinatorRefreshTests.swift +// TableProTests +// +// Tests for handleRefresh, the entry point behind the Refresh toolbar +// button and Cmd+R. Regression coverage for #1637: a refresh issued while +// a query was in flight was silently dropped because the in-flight +// cancellation cleared isExecuting asynchronously. +// + +import Foundation +import Testing + +@testable import TablePro + +@Suite("MainContentCoordinator handleRefresh") +@MainActor +struct MainContentCoordinatorRefreshTests { + private func makeCoordinator() -> (MainContentCoordinator, QueryTabManager) { + let tabManager = QueryTabManager() + let coordinator = MainContentCoordinator( + connection: TestFixtures.makeConnection(), + tabManager: tabManager, + changeManager: DataChangeManager(), + toolbarState: ConnectionToolbarState() + ) + return (coordinator, tabManager) + } + + private func addTableTab( + to tabManager: QueryTabManager, + tableName: String = "users", + query: String = "SELECT * FROM users" + ) -> UUID { + var tab = QueryTab( + title: tableName, + query: query, + tabType: .table, + tableName: tableName + ) + tab.tableContext.isEditable = true + tabManager.tabs.append(tab) + tabManager.selectedTabId = tab.id + return tab.id + } + + private func addQueryTab( + to tabManager: QueryTabManager, + title: String = "Query 1", + query: String = "SELECT 1" + ) -> UUID { + let tab = QueryTab(title: title, query: query, tabType: .query) + tabManager.tabs.append(tab) + tabManager.selectedTabId = tab.id + return tab.id + } + + private func simulateInFlightQuery( + _ coordinator: MainContentCoordinator, + _ tabManager: QueryTabManager, + at index: Int + ) -> Task { + let inFlight = Task { _ = try? await Task.sleep(for: .seconds(60)) } + coordinator.currentQueryTask = inFlight + tabManager.tabs[index].execution.isExecuting = true + tabManager.tabs[index].execution.lastExecutedAt = Date() + return inFlight + } + + @Test("Refresh while a query is in flight cancels it and starts a new execution") + func refreshWithInFlightQueryStartsNewExecution() { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addTableTab(to: tabManager) + guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { + Issue.record("expected tab to exist") + return + } + let staleTask = simulateInFlightQuery(coordinator, tabManager, at: idx) + let initialGeneration = coordinator.queryGeneration + + coordinator.handleRefresh(hasPendingTableOps: false, onDiscard: {}) + defer { coordinator.currentQueryTask?.cancel() } + + #expect(staleTask.isCancelled == true) + #expect(coordinator.queryGeneration == initialGeneration + 2) + #expect(coordinator.currentQueryTask != nil) + #expect(tabManager.tabs[idx].execution.isExecuting == true) + } + + @Test("Refresh on an idle table tab starts an execution") + func refreshOnIdleTabStartsExecution() { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addTableTab(to: tabManager) + guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { + Issue.record("expected tab to exist") + return + } + tabManager.tabs[idx].execution.lastExecutedAt = Date() + let initialGeneration = coordinator.queryGeneration + + coordinator.handleRefresh(hasPendingTableOps: false, onDiscard: {}) + defer { coordinator.currentQueryTask?.cancel() } + + #expect(coordinator.queryGeneration == initialGeneration + 2) + #expect(coordinator.currentQueryTask != nil) + #expect(tabManager.tabs[idx].execution.isExecuting == true) + } + + @Test("Refresh rebuilds the table query from current state before executing") + func refreshRebuildsQuery() { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addTableTab(to: tabManager, query: "SELECT outdated FROM users") + guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { + Issue.record("expected tab to exist") + return + } + tabManager.tabs[idx].execution.lastExecutedAt = Date() + + coordinator.handleRefresh(hasPendingTableOps: false, onDiscard: {}) + defer { coordinator.currentQueryTask?.cancel() } + + #expect(tabManager.tabs[idx].content.query != "SELECT outdated FROM users") + #expect(tabManager.tabs[idx].content.query.contains("users")) + } + + @Test("Refresh on a query tab does not execute or cancel anything") + func refreshOnQueryTabIsNoOp() { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addQueryTab(to: tabManager) + guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { + Issue.record("expected tab to exist") + return + } + let inFlight = Task { _ = try? await Task.sleep(for: .seconds(60)) } + defer { inFlight.cancel() } + coordinator.currentQueryTask = inFlight + tabManager.tabs[idx].execution.isExecuting = true + let initialGeneration = coordinator.queryGeneration + + coordinator.handleRefresh(hasPendingTableOps: false, onDiscard: {}) + + #expect(inFlight.isCancelled == false) + #expect(coordinator.queryGeneration == initialGeneration) + #expect(tabManager.tabs[idx].execution.isExecuting == true) + } + + @Test("Refresh in structure view leaves execution state untouched") + func refreshInStructureViewIsNoOp() { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addTableTab(to: tabManager) + guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { + Issue.record("expected tab to exist") + return + } + tabManager.tabs[idx].display.resultsViewMode = .structure + let staleTask = simulateInFlightQuery(coordinator, tabManager, at: idx) + defer { staleTask.cancel() } + let initialGeneration = coordinator.queryGeneration + + coordinator.handleRefresh(hasPendingTableOps: false, onDiscard: {}) + + #expect(staleTask.isCancelled == false) + #expect(coordinator.queryGeneration == initialGeneration) + #expect(tabManager.tabs[idx].execution.isExecuting == true) + } +}