From 2d8955d1f57013d7eaf1f07d4e95b2bf9829ca36 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 26 Oct 2026 17:40:10 +0700 Subject: [PATCH 1/2] fix(tabs): keep a table tab's rows until you reload it --- .../GridDisplayCacheBudget.swift | 18 + .../MemoryPressureAdvisor.swift | 57 --- .../GridDisplayCacheBudgetTests.swift | 27 ++ .../MemoryPressureAdvisorTests.swift | 31 -- .../Core/Diagnostics/TabSwitchTrace.swift | 2 +- .../MainSplitViewController.swift | 16 +- .../Infrastructure/PostLaunchWork.swift | 3 - apple/TablePro/Models/Query/QueryTab.swift | 4 - apple/TablePro/Models/Query/TabSession.swift | 7 +- .../Models/Query/TabSessionRegistry.swift | 35 +- .../Models/Query/TableLoadTrigger.swift | 6 + apple/TablePro/Models/Query/TableRows.swift | 7 - .../Main/Child/MainEditorContentView.swift | 11 +- .../MainContentCoordinator+GridModels.swift | 4 +- .../MainContentCoordinator+QueryHelpers.swift | 2 +- .../MainContentCoordinator+QueryTasks.swift | 2 +- ...MainContentCoordinator+ResultEditing.swift | 2 + .../MainContentCoordinator+TabRestore.swift | 2 +- .../MainContentCoordinator+TabSwitch.swift | 101 +---- ...nContentCoordinator+TableRowsRefresh.swift | 16 +- ...inContentCoordinator+WindowLifecycle.swift | 37 +- .../Extensions/MainContentView+Helpers.swift | 23 +- .../Extensions/MainContentView+Setup.swift | 2 +- .../Views/Main/MainContentCoordinator.swift | 12 +- .../WorkspaceSwitchKeyStateTests.swift | 6 +- .../TabSessionRegistryTableRowsTests.swift | 44 +-- .../Query/TabSessionRegistryTests.swift | 35 +- .../Models/Query/TabSessionTests.swift | 1 - .../Query/TableRowsRowLocatorTests.swift | 7 - .../Models/Query/TableRowsTests.swift | 17 - .../Views/Main/CatalogChangeWindowTests.swift | 4 +- .../Views/Main/EvictionTests.swift | 357 ------------------ ...nContentCoordinatorDisplayStateTests.swift | 14 - .../MainContentCoordinatorLazyLoadTests.swift | 179 +++++---- .../Main/TableTabRowRetentionTests.swift | 145 +++++++ 35 files changed, 368 insertions(+), 868 deletions(-) create mode 100644 apple/Packages/TableProMacCore/Sources/TableProUtilities/GridDisplayCacheBudget.swift delete mode 100644 apple/Packages/TableProMacCore/Sources/TableProUtilities/MemoryPressureAdvisor.swift create mode 100644 apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/GridDisplayCacheBudgetTests.swift delete mode 100644 apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/MemoryPressureAdvisorTests.swift delete mode 100644 apple/TableProTests/Views/Main/EvictionTests.swift create mode 100644 apple/TableProTests/Views/Main/TableTabRowRetentionTests.swift diff --git a/apple/Packages/TableProMacCore/Sources/TableProUtilities/GridDisplayCacheBudget.swift b/apple/Packages/TableProMacCore/Sources/TableProUtilities/GridDisplayCacheBudget.swift new file mode 100644 index 0000000000..51b940fbf2 --- /dev/null +++ b/apple/Packages/TableProMacCore/Sources/TableProUtilities/GridDisplayCacheBudget.swift @@ -0,0 +1,18 @@ +import Foundation + +// Sized by RAM and never by pressure: dispatch/source.h says not to walk and discard existing caches when pressure rises. +public enum GridDisplayCacheBudget { + public static func unmountedGrids(physicalMemory: UInt64 = ProcessInfo.processInfo.physicalMemory) -> Int { + let gigabyte: UInt64 = 1_073_741_824 + switch physicalMemory { + case (32 * gigabyte)...: + return 8 + case (16 * gigabyte)...: + return 5 + case (8 * gigabyte)...: + return 3 + default: + return 2 + } + } +} diff --git a/apple/Packages/TableProMacCore/Sources/TableProUtilities/MemoryPressureAdvisor.swift b/apple/Packages/TableProMacCore/Sources/TableProUtilities/MemoryPressureAdvisor.swift deleted file mode 100644 index cc10153b63..0000000000 --- a/apple/Packages/TableProMacCore/Sources/TableProUtilities/MemoryPressureAdvisor.swift +++ /dev/null @@ -1,57 +0,0 @@ -import Foundation -import os - -@MainActor -public enum MemoryPressureAdvisor { - nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "MemoryPressureAdvisor") - - /// Current memory pressure level from the OS dispatch source. - private(set) static var isUnderPressure = false - - private static let pressureSource: DispatchSourceMemoryPressure = { - let source = DispatchSource.makeMemoryPressureSource( - eventMask: [.warning, .critical, .normal], - queue: .main - ) - source.setEventHandler { @Sendable in - MainActor.assumeIsolated { - let event = source.data - let wasPressured = isUnderPressure - isUnderPressure = event.contains(.warning) || event.contains(.critical) - if isUnderPressure && !wasPressured { - logger.info("Memory pressure detected, reducing tab eviction budget") - } else if !isUnderPressure && wasPressured { - logger.info("Memory pressure resolved, restoring tab eviction budget") - } - } - } - source.activate() - return source - }() - - public static func startMonitoring() { - _ = pressureSource - } - - public static func budgetForInactiveTabs() -> Int { - let totalBytes = ProcessInfo.processInfo.physicalMemory - let gb: UInt64 = 1_073_741_824 - - let baseBudget: Int - if totalBytes >= 32 * gb { - baseBudget = 8 - } else if totalBytes >= 16 * gb { - baseBudget = 5 - } else if totalBytes >= 8 * gb { - baseBudget = 3 - } else { - baseBudget = 2 - } - - return isUnderPressure ? max(1, baseBudget / 2) : baseBudget - } - - public static func estimatedFootprint(rowCount: Int, columnCount: Int) -> Int { - rowCount * columnCount * 64 - } -} diff --git a/apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/GridDisplayCacheBudgetTests.swift b/apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/GridDisplayCacheBudgetTests.swift new file mode 100644 index 0000000000..0043289325 --- /dev/null +++ b/apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/GridDisplayCacheBudgetTests.swift @@ -0,0 +1,27 @@ +import Testing + +@testable import TableProUtilities + +struct GridDisplayCacheBudgetTests { + private static let gigabyte: UInt64 = 1_073_741_824 + + @Test( + "More memory keeps more unmounted grids formatted", + arguments: [ + (UInt64(4) * gigabyte, 2), + (UInt64(8) * gigabyte, 3), + (UInt64(16) * gigabyte, 5), + (UInt64(24) * gigabyte, 5), + (UInt64(32) * gigabyte, 8), + (UInt64(128) * gigabyte, 8), + ] + ) + func tiers(physicalMemory: UInt64, expected: Int) { + #expect(GridDisplayCacheBudget.unmountedGrids(physicalMemory: physicalMemory) == expected) + } + + @Test("Just under a tier stays on the lower one") + func tierBoundary() { + #expect(GridDisplayCacheBudget.unmountedGrids(physicalMemory: 16 * Self.gigabyte - 1) == 3) + } +} diff --git a/apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/MemoryPressureAdvisorTests.swift b/apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/MemoryPressureAdvisorTests.swift deleted file mode 100644 index f22ecebd86..0000000000 --- a/apple/Packages/TableProMacCore/Tests/TableProUtilitiesTests/MemoryPressureAdvisorTests.swift +++ /dev/null @@ -1,31 +0,0 @@ -import Testing - -@testable import TableProUtilities - -@MainActor -struct MemoryPressureAdvisorTests { - @Test("budget returns positive value") - func budgetPositive() { - let budget = MemoryPressureAdvisor.budgetForInactiveTabs() - #expect(budget >= 2) - #expect(budget <= 8) - } - - @Test("memory estimation for typical tab") - func typicalTabEstimate() { - let bytes = MemoryPressureAdvisor.estimatedFootprint(rowCount: 1_000, columnCount: 10) - #expect(bytes == 640_000) - } - - @Test("memory estimation for empty tab") - func emptyTabEstimate() { - let bytes = MemoryPressureAdvisor.estimatedFootprint(rowCount: 0, columnCount: 10) - #expect(bytes == 0) - } - - @Test("memory estimation for large tab") - func largeTabEstimate() { - let bytes = MemoryPressureAdvisor.estimatedFootprint(rowCount: 50_000, columnCount: 20) - #expect(bytes == 64_000_000) - } -} diff --git a/apple/TablePro/Core/Diagnostics/TabSwitchTrace.swift b/apple/TablePro/Core/Diagnostics/TabSwitchTrace.swift index 815d794304..ec160af1d6 100644 --- a/apple/TablePro/Core/Diagnostics/TabSwitchTrace.swift +++ b/apple/TablePro/Core/Diagnostics/TabSwitchTrace.swift @@ -12,7 +12,7 @@ internal enum TabSwitchTrigger: String, Sendable { case tabStrip } -// requery: the memory budget dropped the rows. firstLoad: restored from the last session, never shown since. +// requery: TablePro changed the table since the rows were read. firstLoad: restored from the last session, never shown since. internal enum TabSwitchRows: String, Sendable { case resident case requery diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index d5e220c969..7b74595818 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -650,15 +650,8 @@ internal final class MainSplitViewController: NSSplitViewController { /// Switching workspace repaints the window in place. The rail used to raise a different /// window instead, which is what made several connections mean several windows. internal func applySelectedWorkspace() { - /// Switching workspace is this window's key-window change as far as a coordinator is - /// concerned. Only the selected one receives the real `windowDidBecomeKey`, so without - /// this the outgoing connection keeps `isKeyWindow` true and never schedules the eviction - /// that frees its row buffers. - /// - /// The outgoing side is read from each coordinator rather than from `lastActiveCoordinator`, - /// which only a switch sets. The window's first connection is made key by the window itself, - /// so the first switch away from it found no cached coordinator to resign: it kept - /// `isKeyWindow` and never scheduled the eviction. + // Only the selected connection gets the real windowDidBecomeKey, so a switch hands key status over itself. + // Each coordinator is asked, not `lastActiveCoordinator`: the window made its first connection key without a switch. let incoming = workspaces.selected?.sessionState?.coordinator var handedOffKey = false for workspace in workspaces.workspaces { @@ -668,10 +661,7 @@ internal final class MainSplitViewController: NSSplitViewController { coordinator.handleWindowDidResignKey() handedOffKey = true } - /// Key status is handed over, never invented. A window that is not key can still change its - /// selection, and marking that connection key kept its row eviction from ever being - /// scheduled and let a deferred restore load in a window nobody was looking at. The real - /// `windowDidBecomeKey` reaches whichever connection is selected when the window comes forward. + // Handed over, never invented: marking a connection key in a background window ran its deferred restore load unseen. if lastActiveCoordinator !== incoming { if handedOffKey || view.window?.isKeyWindow == true { incoming?.handleWindowDidBecomeKey() diff --git a/apple/TablePro/Core/Services/Infrastructure/PostLaunchWork.swift b/apple/TablePro/Core/Services/Infrastructure/PostLaunchWork.swift index c85b3461fe..055a6197b2 100644 --- a/apple/TablePro/Core/Services/Infrastructure/PostLaunchWork.swift +++ b/apple/TablePro/Core/Services/Infrastructure/PostLaunchWork.swift @@ -1,6 +1,5 @@ import Foundation import os -import TableProUtilities /// The subsystem work a launch has to do but the first window does not need. /// @@ -28,8 +27,6 @@ internal enum PostLaunchWork { hasStarted = true logger.debug("post-launch work starting") - MemoryPressureAdvisor.startMonitoring() - Task { await PluginManager.shared.sweepPluginSignatures() } Task { await QueryHistoryManager.shared.performStartupCleanup() } diff --git a/apple/TablePro/Models/Query/QueryTab.swift b/apple/TablePro/Models/Query/QueryTab.swift index bdcbb9fc3d..072966f0b8 100644 --- a/apple/TablePro/Models/Query/QueryTab.swift +++ b/apple/TablePro/Models/Query/QueryTab.swift @@ -71,7 +71,6 @@ struct QueryTab: Identifiable, Equatable { var schemaVersion: Int var metadataVersion: Int var paginationVersion: Int - var loadEpoch: Int = 0 var pendingRestoredSort: [PersistedSortColumn]? // Held until the first load resolves the columns. @@ -155,7 +154,6 @@ struct QueryTab: Identifiable, Equatable { self.schemaVersion = 0 self.metadataVersion = 0 self.paginationVersion = 0 - self.loadEpoch = 0 self.pendingRestoredSort = nil self.restoredSortSource = .unset self.restoredPage = nil @@ -210,7 +208,6 @@ struct QueryTab: Identifiable, Equatable { self.schemaVersion = 0 self.metadataVersion = 0 self.paginationVersion = 0 - self.loadEpoch = 0 self.pendingRestoredSort = persisted.sortColumns // A file from before sortSource decodes to its old behaviour: saved columns were the user's, none meant undecided. self.restoredSortSource = persisted.sortSource @@ -387,6 +384,5 @@ struct QueryTab: Identifiable, Equatable { && lhs.isPreview == rhs.isPreview && lhs.isPinned == rhs.isPinned && lhs.hasUserInteraction == rhs.hasUserInteraction - && lhs.loadEpoch == rhs.loadEpoch } } diff --git a/apple/TablePro/Models/Query/TabSession.swift b/apple/TablePro/Models/Query/TabSession.swift index 9038cd12f8..7d926a3d79 100644 --- a/apple/TablePro/Models/Query/TabSession.swift +++ b/apple/TablePro/Models/Query/TabSession.swift @@ -24,7 +24,6 @@ final class TabSession: ObservableObject, Identifiable { let id: UUID @Published var tableRows: TableRows - @Published var isEvicted: Bool /// Bumped by `TabSessionRegistry` on every mutation of `tableRows`. `TableRows` is a /// value type with no `Equatable` conformance, so views that derive expensive content @@ -48,15 +47,13 @@ final class TabSession: ObservableObject, Identifiable { var viewportStage: GridViewportStage? /// Whether the table changed after these rows, or the definition they were read with, were - /// fetched. Unlike eviction the rows stay, so the grid goes on showing them until the reload - /// commits over them, and a tab whose last result was empty can be marked too. Not published: - /// nothing draws it. + /// fetched. The rows stay, so the grid goes on showing them until the reload commits over them, + /// and a tab whose last result was empty can be marked too. Not published: nothing draws it. var freshness: TableFreshness init(id: UUID = UUID()) { self.id = id self.tableRows = TableRows() - self.isEvicted = false self.freshness = TableFreshness() self.dataRevision = 0 self.bufferEpoch = 0 diff --git a/apple/TablePro/Models/Query/TabSessionRegistry.swift b/apple/TablePro/Models/Query/TabSessionRegistry.swift index 6479ac81b0..6b2429bd89 100644 --- a/apple/TablePro/Models/Query/TabSessionRegistry.swift +++ b/apple/TablePro/Models/Query/TabSessionRegistry.swift @@ -40,29 +40,17 @@ final class TabSessionRegistry { func setTableRows(_ rows: TableRows, for tabId: UUID) { let session = ensureSession(for: tabId) session.tableRows = rows - session.isEvicted = false session.dataRevision &+= 1 session.bufferEpoch &+= 1 session.rowSetRevision &+= 1 } - /// A mutation of what the tab already holds, so it cannot resurrect a tab that holds nothing. - /// - /// Phase-2 metadata (foreign keys, defaults, enum values) lands at background priority long - /// after the load that asked for it, and it is keyed on the result rather than on the rows, so - /// it arrives on tabs that were evicted while it was in flight. Clearing `isEvicted` for a - /// mutation that leaves the buffer empty leaves a tab with no rows that `canAutoLoadTableTab` - /// reads as already loaded, and eviction cannot re-mark it because it has nothing left to lose: - /// the grid stays empty until an explicit refresh. @discardableResult func updateTableRows(for tabId: UUID, _ mutate: (inout TableRows) -> Delta) -> Delta { let session = ensureSession(for: tabId) var rows = session.tableRows let delta = mutate(&rows) session.tableRows = rows - if !rows.rows.isEmpty { - session.isEvicted = false - } session.dataRevision &+= 1 if delta.changesRowSet { session.rowSetRevision &+= 1 @@ -73,26 +61,6 @@ final class TabSessionRegistry { func removeTableRows(for tabId: UUID) { guard let session = sessions[tabId] else { return } session.tableRows = TableRows() - session.isEvicted = false - session.dataRevision &+= 1 - session.bufferEpoch &+= 1 - session.rowSetRevision &+= 1 - } - - func isEvicted(_ tabId: UUID) -> Bool { - sessions[tabId]?.isEvicted ?? false - } - - /// Drops a tab's rows and marks it evicted. Getting them back is the caller's job: SwiftUI's - /// `.task(id:)` keys on `QueryTab.loadEpoch`, so a caller that wants the lazy load to re-fire - /// bumps that itself. - /// - /// A tab with no rows is left alone, so eviction never marks a tab that has nothing to lose. - func evict(for tabId: UUID) { - guard let session = sessions[tabId] else { return } - guard !session.tableRows.rows.isEmpty else { return } - session.tableRows.discardRowsKeepingMetadata() - session.isEvicted = true session.dataRevision &+= 1 session.bufferEpoch &+= 1 session.rowSetRevision &+= 1 @@ -109,8 +77,7 @@ final class TabSessionRegistry { } /// Records that the tab's table changed. Nothing is discarded, and a tab holding no rows is - /// marked all the same: an empty collection that gains its first document is exactly the tab - /// eviction refuses. + /// marked all the same: an empty collection can gain its first document. func recordChange(_ change: TableFreshness.Change, for tabId: UUID) { ensureSession(for: tabId).freshness.record(change) } diff --git a/apple/TablePro/Models/Query/TableLoadTrigger.swift b/apple/TablePro/Models/Query/TableLoadTrigger.swift index 240a04661d..744f343fd2 100644 --- a/apple/TablePro/Models/Query/TableLoadTrigger.swift +++ b/apple/TablePro/Models/Query/TableLoadTrigger.swift @@ -9,3 +9,9 @@ internal enum TableLoadTrigger { case userInitiated case restore } + +// Keyed by tab: a window-wide trigger was consumed against whatever tab was selected when the connection came back. +internal struct ParkedTableLoad: Equatable { + let tabId: UUID + let trigger: TableLoadTrigger +} diff --git a/apple/TablePro/Models/Query/TableRows.swift b/apple/TablePro/Models/Query/TableRows.swift index 852751660b..b09235c46c 100644 --- a/apple/TablePro/Models/Query/TableRows.swift +++ b/apple/TablePro/Models/Query/TableRows.swift @@ -174,13 +174,6 @@ struct TableRows: Sendable { rowLocators[id] } - /// Releases the row payload while keeping the schema needed to render and reload the table. - mutating func discardRowsKeepingMetadata() { - rows = [] - indexByID = [:] - rowLocators = [:] - } - /// Writes a value into a cell, or with `isAbsent` takes the field out of the row. A value /// written into a missing field puts the field back. @discardableResult diff --git a/apple/TablePro/Views/Main/Child/MainEditorContentView.swift b/apple/TablePro/Views/Main/Child/MainEditorContentView.swift index 366fd67835..dda24f010d 100644 --- a/apple/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/apple/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -11,12 +11,6 @@ import SwiftUI import TableProEditorKit import TableProPluginKit -// A change to either field cancels the old task and starts a new one, which coalesces rapid Cmd+Number switches. -private struct TabLoadKey: Hashable { - let tabId: UUID? - let loadEpoch: Int -} - // Only a tab showing rows reruns: an error, a plan or a write's count is not a view of the new database's data. @MainActor internal enum ContainerChangeRerunPolicy { @@ -169,10 +163,7 @@ struct MainEditorContentView: View { .onAppear { updateHasQueryText() } - .task(id: TabLoadKey( - tabId: tabManager.selectedTabId, - loadEpoch: tabManager.selectedTab?.loadEpoch ?? 0 - )) { + .task(id: tabManager.selectedTabId) { coordinator.lazyLoadCurrentTabIfNeeded() } } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+GridModels.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+GridModels.swift index 268fc780cf..ad9c2ffc2a 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+GridModels.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+GridModels.swift @@ -235,9 +235,9 @@ extension MainContentCoordinator { dataGridEntries[tabId]?.displayInputs = inputs } - // Each model holds a whole result's formatted text, so only the tabs in use keep theirs. + // The only memory a tab switch releases: formatted text, rebuilt per drawn cell. Rows stay until the user replaces them. func pruneGridDisplayCaches() { - let budget = MemoryPressureAdvisor.budgetForInactiveTabs() + 1 + let budget = GridDisplayCacheBudget.unmountedGrids() + 1 guard dataGridEntries.count > budget else { return } let ordered = dataGridEntries.sorted { $0.value.lastUsed > $1.value.lastUsed } for (tabId, entry) in ordered.dropFirst(budget) where !entry.model.isMounted { diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift index fd76bf1db0..1ca309a771 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift @@ -76,7 +76,7 @@ extension MainContentCoordinator { return } if isAutoLoad, services.databaseManager.driver(for: connectionId)?.status != .connected { - pendingLoadTrigger = trigger + parkedTableLoad = ParkedTableLoad(tabId: tabId, trigger: trigger) return } handleQueryExecutionError(error, sql: sql, tabId: tabId, connection: conn, serverOutput: serverOutput) diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryTasks.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryTasks.swift index eae085f421..e3cfd3d489 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryTasks.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryTasks.swift @@ -24,7 +24,7 @@ extension MainContentCoordinator { traceConnectUnavailable(traceToken) guard tabExecution.settle(claim) else { return false } retireQueryTask(.claim(claim)) - pendingLoadTrigger = trigger + parkedTableLoad = ParkedTableLoad(tabId: claim.tabId, trigger: trigger) declineTableLoad(for: claim.tabId) return false } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+ResultEditing.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+ResultEditing.swift index 47db884572..3062fb2c1a 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+ResultEditing.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+ResultEditing.swift @@ -29,9 +29,11 @@ extension MainContentCoordinator { /// two. They all read this now. var canEditActiveResult: Bool { guard let tab = tabManager.selectedTab else { return false } + // A claimed execution replaces these rows and clears whatever was staged on them meanwhile, as the data files grid does when busy. return tab.tableContext.isEditable && tab.tableContext.allowsRowEditing && !safeModeLevel.blocksAllWrites && activeResultEditRefusal == nil + && !tabExecution.isExecuting(tab.id) } } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabRestore.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabRestore.swift index bb1290d0c5..b4f223e6a6 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabRestore.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabRestore.swift @@ -37,7 +37,7 @@ extension MainContentCoordinator { } guard let session = DatabaseManager.shared.activeSessions[connection.id], session.isConnected else { - if isTableTab { pendingLoadTrigger = .restore } + if isTableTab { parkedTableLoad = ParkedTableLoad(tabId: selected.id, trigger: .restore) } return } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift index d89a25e631..00257963f2 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift @@ -9,7 +9,6 @@ import Foundation import os import TableProEditorKit -import TableProUtilities extension MainContentCoordinator { func handleTabChange( @@ -63,13 +62,9 @@ extension MainContentCoordinator { } let saveMs = Int(Date().timeIntervalSince(saveStart) * 1_000) - // Deferred a tick: the sort and budget calculation is costly with many open tabs. - if tabManager.tabs.count > 2 { - let activeIds: Set = Set([oldTabId, newTabId].compactMap { $0 }) - Task { @MainActor [weak self] in - self?.evictInactiveTabs(excluding: activeIds) - self?.pruneGridDisplayCaches() - } + // Deferred a tick: releasing a large formatted cache is not work the switch should wait on. + Task { @MainActor [weak self] in + self?.pruneGridDisplayCaches() } let restoreStart = Date() @@ -117,11 +112,11 @@ extension MainContentCoordinator { ) // No `reloadVersion` bump: it is window-wide, and bumping it on a switch re-formatted the incoming grid's whole result (#2424). - let needsReload = needsRowReload(newId) + let isStale = tabSessionRegistry.isStale(newId) lazyLoadCurrentTabIfNeeded() TabSwitchTracer.shared.selectionApplied( tabId: newId, - rows: TabSwitchRows(loads: isLoadingRows(newId), needsReload: needsReload), + rows: TabSwitchRows(loads: isLoadingRows(newId), needsReload: isStale), window: contentWindow, isShowing: { [weak manager = tabManager] in manager?.isShowing(newId) ?? false } ) @@ -153,14 +148,10 @@ extension MainContentCoordinator { guard let tab = tabManager.tabs.first(where: { $0.id == tabId }) else { return .resident } return TabSwitchRows( loads: isLoadingRows(tabId) || canAutoLoadTableTab(tab), - needsReload: needsRowReload(tabId) + needsReload: tabSessionRegistry.isStale(tabId) ) } - private func needsRowReload(_ tabId: UUID) -> Bool { - tabSessionRegistry.isEvicted(tabId) || tabSessionRegistry.isStale(tabId) - } - // Only a traced load reports its end; a multi-statement run does not, and would hold the switch open. private func isLoadingRows(_ tabId: UUID) -> Bool { TableLoadTracer.shared.activeToken(for: tabId) != nil @@ -185,84 +176,4 @@ extension MainContentCoordinator { guard let tabId = tabManager.selectedTabId else { return false } return tabExecution.isExecuting(tabId) } - - // Must agree with `canAutoLoadTableTab`: a tab evicted with no route back to its rows shows an empty grid until refreshed. - func canEvictReloadableTableRows(_ tab: QueryTab) -> Bool { - guard tab.id != tabManager.selectedTabId, - tab.tabType == .table, - tab.execution.errorMessage == nil, - tab.content.query.contains(where: { !$0.isWhitespace }), - !tab.pendingChanges.hasChanges, - !tab.display.hasPinnedResults, - !tab.pagination.isLoading, - !tab.pagination.isLoadingMore, - !tabExecution.isBusy(tab.id), - tableLoadTasks[tab.id] == nil, - !tabSessionRegistry.isEvicted(tab.id), - let rows = tabSessionRegistry.existingTableRows(for: tab.id), - !rows.rows.isEmpty - else { return false } - return true - } - - @discardableResult - func evictReloadableTableRows(for tabId: UUID) -> Bool { - guard let index = tabManager.tabs.firstIndex(where: { $0.id == tabId }), - canEvictReloadableTableRows(tabManager.tabs[index]) - else { return false } - - tabManager.mutate(at: index) { tab in - for resultSet in tab.display.resultSets where !resultSet.isPinned { - resultSet.tableRows.discardRowsKeepingMetadata() - } - tab.loadEpoch &+= 1 - } - tabSessionRegistry.evict(for: tabId) - reloadGridModel(forTab: tabId) - return true - } - - func evictInactiveTabs(excluding activeTabIds: Set) { - let start = Date() - let candidates: [(tab: QueryTab, rows: TableRows)] = tabManager.tabs.compactMap { tab in - guard !activeTabIds.contains(tab.id), - tab.execution.lastExecutedAt != nil, - canEvictReloadableTableRows(tab), - let rows = tabSessionRegistry.existingTableRows(for: tab.id) - else { return nil } - return (tab, rows) - } - - let sorted = candidates.sorted { - let t0 = $0.tab.execution.lastExecutedAt ?? .distantFuture - let t1 = $1.tab.execution.lastExecutedAt ?? .distantFuture - if t0 != t1 { return t0 < t1 } - let size0 = MemoryPressureAdvisor.estimatedFootprint( - rowCount: $0.rows.rows.count, - columnCount: $0.rows.columns.count - ) - let size1 = MemoryPressureAdvisor.estimatedFootprint( - rowCount: $1.rows.rows.count, - columnCount: $1.rows.columns.count - ) - return size0 > size1 - } - - let maxInactiveLoaded = MemoryPressureAdvisor.budgetForInactiveTabs() - guard sorted.count > maxInactiveLoaded else { - Self.lifecycleLogger.debug( - "[switch] evictInactiveTabs no-op candidates=\(sorted.count) budget=\(maxInactiveLoaded) elapsedMs=\(Int(Date().timeIntervalSince(start) * 1_000))" - ) - return - } - let toEvict = sorted.dropLast(maxInactiveLoaded) - - var evicted = 0 - for entry in toEvict { - if evictReloadableTableRows(for: entry.tab.id) { evicted += 1 } - } - Self.lifecycleLogger.debug( - "[switch] evictInactiveTabs evicted=\(evicted) attempted=\(toEvict.count) candidates=\(sorted.count) keptInactive=\(maxInactiveLoaded) elapsedMs=\(Int(Date().timeIntervalSince(start) * 1_000))" - ) - } } diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TableRowsRefresh.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TableRowsRefresh.swift index 74b9acd5d0..66afcadb7a 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TableRowsRefresh.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+TableRowsRefresh.swift @@ -119,10 +119,18 @@ extension MainContentCoordinator { private var selectedTabRefreshState: TableRowsRefreshPlan.SelectedTabState? { guard let tab = tabManager.selectedTab else { return nil } - let holdsEdits = changeManager.hasChanges - || tab.pendingChanges.hasChanges - || isEditingActiveGrid - return TableRowsRefreshPlan.SelectedTabState(id: tab.id, holdsEdits: holdsEdits, load: selectedTabLoad(tab.id)) + return TableRowsRefreshPlan.SelectedTabState( + id: tab.id, + holdsEdits: holdsUnsavedWork(tab), + load: selectedTabLoad(tab.id) + ) + } + + // The one answer for every reload path, the lazy load included: an open cell counts, or a reload drops what is typed in it. + func holdsUnsavedWork(_ tab: QueryTab) -> Bool { + if tab.pendingChanges.hasChanges { return true } + guard tab.id == changeManagerTabId else { return false } + return changeManager.hasChanges || isEditingActiveGrid } private func selectedTabLoad(_ tabId: UUID) -> TableRowsRefreshPlan.SelectedTabLoad { diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+WindowLifecycle.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+WindowLifecycle.swift index d19feeaa42..da40de84c0 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+WindowLifecycle.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+WindowLifecycle.swift @@ -20,8 +20,6 @@ extension MainContentCoordinator { "[switch] coordinator.handleWindowDidBecomeKey connId=\(self.connectionId, privacy: .public) selectedTabId=\(self.tabManager.selectedTabId?.uuidString ?? "nil", privacy: .public)" ) isKeyWindow = true - evictionTask?.cancel() - evictionTask = nil consumeDeferredRestoreLoadIfNeeded() refreshSourceFileDiskChanges() @@ -40,16 +38,6 @@ extension MainContentCoordinator { "[switch] coordinator.handleWindowDidResignKey connId=\(self.connectionId, privacy: .public)" ) isKeyWindow = false - - evictionTask?.cancel() - evictionTask = Task { [weak self] in - try? await Task.sleep(for: .seconds(5)) - guard let self, !Task.isCancelled else { return } - Self.lifecycleLogger.debug( - "[switch] coordinator evictInactiveRowData firing (5s after resignKey) connId=\(self.connectionId, privacy: .public)" - ) - self.evictInactiveRowData() - } } // Synchronous on purpose: no grace period, no delayed Task. @@ -65,9 +53,6 @@ extension MainContentCoordinator { persistence.saveAggregatedSync() } - evictionTask?.cancel() - evictionTask = nil - trailingPaneState?.teardown() teardown() @@ -175,7 +160,7 @@ extension MainContentCoordinator { session.isConnected else { tracer.anomaly(.connectionNotReady, token: traceToken) tracer.finish(token: traceToken, outcome: .notConnected) - pendingLoadTrigger = trigger + parkedTableLoad = ParkedTableLoad(tabId: tab.id, trigger: trigger) declineTableLoad(for: tab.id) return } @@ -211,19 +196,25 @@ extension MainContentCoordinator { lazyLoadCurrentTabIfNeeded(trigger: .restore) } + // The parked trigger goes only to the tab that parked it: run against another selected tab, it executed a query tab's SQL. + func loadSelectedTableTabAfterConnect() { + let parked = parkedTableLoad + parkedTableLoad = nil + let trigger = parked.flatMap { $0.tabId == tabManager.selectedTabId ? $0.trigger : nil } + lazyLoadCurrentTabIfNeeded(trigger: trigger ?? .userInitiated) + } + + // Rows a tab holds stay until the user asks for others; only a first load or a change TablePro made to the table loads here. func canAutoLoadTableTab(_ tab: QueryTab) -> Bool { guard tab.tabType == .table else { return false } guard tab.execution.errorMessage == nil else { return false } guard !tab.content.query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { return false } - let rows = tabSessionRegistry.tableRows(for: tab.id) - let needsReload = tabSessionRegistry.isEvicted(tab.id) || tabSessionRegistry.isStale(tab.id) - let hasFreshRows = !rows.rows.isEmpty && !needsReload - let hasExecuted = tab.execution.lastExecutedAt != nil && !needsReload - guard !hasFreshRows, !hasExecuted else { return false } + let hasLoaded = tab.execution.lastExecutedAt != nil + || !tabSessionRegistry.tableRows(for: tab.id).rows.isEmpty + guard !hasLoaded || tabSessionRegistry.isStale(tab.id) else { return false } - let hasPendingEdits = changeManager.hasChanges || tab.pendingChanges.hasChanges - return !hasPendingEdits + return !holdsUnsavedWork(tab) } private func clearAbandonedExecutingFlagIfNeeded(for tab: QueryTab) { diff --git a/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift b/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift index 4885f746c6..5e148f871e 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentView+Helpers.swift @@ -31,17 +31,7 @@ extension MainContentView { let sessions = DatabaseManager.shared.activeSessions guard let session = sessions[connection.id] else { return } if session.isConnected { - if let trigger = coordinator.pendingLoadTrigger { - let hasPendingEdits = - changeManager.hasChanges - || (tabManager.selectedTab?.pendingChanges.hasChanges ?? false) - if !hasPendingEdits { - coordinator.pendingLoadTrigger = nil - consumePendingLoad(trigger: trigger) - } - } else { - coordinator.lazyLoadCurrentTabIfNeeded() - } + coordinator.loadSelectedTableTabAfterConnect() } toolbarState.updateConnectionState(from: session.reportedStatus) toolbarState.syncFromSession(for: connection) @@ -55,17 +45,6 @@ extension MainContentView { ) } - private func consumePendingLoad(trigger: TableLoadTrigger) { - if let tabId = tabManager.selectedTab?.id { - coordinator.resolveTableTabSchemaIfNeeded(tabId: tabId) - } - if tabManager.selectedTab?.tabType == .table { - coordinator.lazyLoadCurrentTabIfNeeded(trigger: trigger) - } else { - coordinator.runQuery(viewport: .firstRow, trigger: trigger) - } - } - // MARK: - Inspector Context func scheduleInspectorUpdate() { diff --git a/apple/TablePro/Views/Main/Extensions/MainContentView+Setup.swift b/apple/TablePro/Views/Main/Extensions/MainContentView+Setup.swift index 6d1c4b77d1..84b9a6e042 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentView+Setup.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentView+Setup.swift @@ -68,7 +68,7 @@ extension MainContentView { if let session = DatabaseManager.shared.activeSessions[connection.id], session.isConnected { coordinator.lazyLoadCurrentTabIfNeeded() } else { - coordinator.pendingLoadTrigger = .userInitiated + coordinator.parkedTableLoad = ParkedTableLoad(tabId: selectedTab.id, trigger: .userInitiated) } } diff --git a/apple/TablePro/Views/Main/MainContentCoordinator.swift b/apple/TablePro/Views/Main/MainContentCoordinator.swift index c293e1242e..8659645d71 100644 --- a/apple/TablePro/Views/Main/MainContentCoordinator.swift +++ b/apple/TablePro/Views/Main/MainContentCoordinator.swift @@ -272,7 +272,7 @@ final class MainContentCoordinator: ObservableObject { @Published var exportFormatId: String? // Kept after the sheet closes, so reopening Search Database after opening a result shows the results again. var databaseSearchSession: DatabaseSearchSession? - @Published var pendingLoadTrigger: TableLoadTrigger? + var parkedTableLoad: ParkedTableLoad? var deferredRestoreLoadTabId: UUID? var displayFormatsCache: [UUID: DisplayFormatsCacheEntry] = [:] @@ -373,9 +373,6 @@ final class MainContentCoordinator: ObservableObject { // Updated by TabWindowController; read by event handlers such as the sidebar table-selection filter. var isKeyWindow = false - // Scheduled in `handleWindowDidResignKey`, fires 5s later. - var evictionTask: Task? - var refreshCoalesceTask: Task? var refreshPendingTrailing = false @@ -576,13 +573,6 @@ final class MainContentCoordinator: ObservableObject { } }() - // Called on resign key. The selected tab keeps its rows; `canEvictReloadableTableRows` keeps tabs that can't reload themselves. - func evictInactiveRowData() { - for tab in tabManager.tabs { - evictReloadableTableRows(for: tab.id) - } - } - // MARK: - Initialization init( diff --git a/apple/TableProTests/Core/Services/Infrastructure/WorkspaceSwitchKeyStateTests.swift b/apple/TableProTests/Core/Services/Infrastructure/WorkspaceSwitchKeyStateTests.swift index 15f02c6d28..9fc313ea1b 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/WorkspaceSwitchKeyStateTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/WorkspaceSwitchKeyStateTests.swift @@ -3,8 +3,7 @@ // TableProTests // // Switching connection inside a window is that window's key change as far as each connection's -// coordinator is concerned: the one leaving the screen resigns, which is what schedules the -// eviction of its row buffers. +// coordinator is concerned: the one leaving the screen resigns. // import AppKit @@ -75,12 +74,11 @@ struct WorkspaceSwitchKeyStateTests { harness.controller.workspaces.select(harness.second.connectionId) #expect(first.isKeyWindow == false) - #expect(first.evictionTask != nil) #expect(second.isKeyWindow) } /// A window that is not key can still change its selection, for example when a close is - /// cancelled. Marking that connection key kept its row eviction from ever being scheduled. + /// cancelled. Marking that connection key ran its deferred restore load in a window nobody saw. @Test("Switching in a window that is not key marks nothing key") func switchInNonKeyWindowMarksNothingKey() throws { let harness = Harness() diff --git a/apple/TableProTests/Core/Services/Query/TabSessionRegistryTableRowsTests.swift b/apple/TableProTests/Core/Services/Query/TabSessionRegistryTableRowsTests.swift index 1f34947861..0389fc5ea1 100644 --- a/apple/TableProTests/Core/Services/Query/TabSessionRegistryTableRowsTests.swift +++ b/apple/TableProTests/Core/Services/Query/TabSessionRegistryTableRowsTests.swift @@ -14,7 +14,6 @@ struct TabSessionRegistryTableRowsTests { let first = store.tableRows(for: tabId) #expect(first.rows.isEmpty) #expect(first.columns.isEmpty) - #expect(store.isEvicted(tabId) == false) let second = store.tableRows(for: tabId) #expect(second.rows.count == first.rows.count) @@ -58,7 +57,7 @@ struct TabSessionRegistryTableRowsTests { #expect(resolved?.rows.count == 1) } - @Test("removeTableRows(for:) deletes the entry and clears evicted state") + @Test("removeTableRows(for:) deletes the entry") func removeTableRowsDeletes() { let store = TabSessionRegistry() let tabId = UUID() @@ -67,50 +66,9 @@ struct TabSessionRegistryTableRowsTests { TableRows.from(queryRows: [["x"]], columns: ["c"], columnTypes: [.text(rawType: nil)]), for: tabId ) - store.evict(for: tabId) - #expect(store.isEvicted(tabId) == true) store.removeTableRows(for: tabId) #expect(store.existingTableRows(for: tabId) == nil) - #expect(store.isEvicted(tabId) == false) - } - - @Test("evict(for:) clears rows and marks evicted while preserving columns") - func evictMarksEvicted() { - let store = TabSessionRegistry() - let tabId = UUID() - let rows = TableRows.from( - queryRows: [["a"], ["b"]], - columns: ["c"], - columnTypes: [.text(rawType: nil)] - ) - store.setTableRows(rows, for: tabId) - - #expect(store.isEvicted(tabId) == false) - store.evict(for: tabId) - - #expect(store.isEvicted(tabId) == true) - let evicted = store.existingTableRows(for: tabId) - #expect(evicted?.rows.isEmpty == true) - #expect(evicted?.columns == ["c"]) - } - - @Test("setTableRows clears evicted flag") - func setClearsEvicted() { - let store = TabSessionRegistry() - let tabId = UUID() - store.setTableRows( - TableRows.from(queryRows: [["a"]], columns: ["c"], columnTypes: [.text(rawType: nil)]), - for: tabId - ) - store.evict(for: tabId) - #expect(store.isEvicted(tabId) == true) - - store.setTableRows( - TableRows.from(queryRows: [["b"]], columns: ["c"], columnTypes: [.text(rawType: nil)]), - for: tabId - ) - #expect(store.isEvicted(tabId) == false) } @Test("updateTableRows applies mutation in place") diff --git a/apple/TableProTests/Models/Query/TabSessionRegistryTests.swift b/apple/TableProTests/Models/Query/TabSessionRegistryTests.swift index a3acc545b7..c13f0553a7 100644 --- a/apple/TableProTests/Models/Query/TabSessionRegistryTests.swift +++ b/apple/TableProTests/Models/Query/TabSessionRegistryTests.swift @@ -182,7 +182,7 @@ struct TabSessionRegistryTests { #expect(session.rowSetRevision == before) } - @Test("Replacing, removing and evicting the buffer each move rowSetRevision") + @Test("Replacing and removing the buffer each move rowSetRevision") func bufferReplacementMovesRowSetRevision() { let registry = TabSessionRegistry() let session = TabSession() @@ -190,15 +190,12 @@ struct TabSessionRegistryTests { registry.setTableRows(makeRows(["a"]), for: session.id) let afterSet = session.rowSetRevision - registry.evict(for: session.id) - let afterEvict = session.rowSetRevision registry.setTableRows(makeRows(["b"]), for: session.id) let afterSecondSet = session.rowSetRevision registry.removeTableRows(for: session.id) #expect(afterSet > 0) - #expect(afterEvict > afterSet) - #expect(afterSecondSet > afterEvict) + #expect(afterSecondSet > afterSet) #expect(session.rowSetRevision > afterSecondSet) } @@ -215,56 +212,32 @@ struct TabSessionRegistryTests { #expect(session.dataRevision > before) } - @Test("evict bumps dataRevision, and leaves it alone when there is nothing to evict") - func evictBumpsDataRevision() { - let registry = TabSessionRegistry() - let session = TabSession() - registry.register(session) - registry.setTableRows(makeRows(["a"]), for: session.id) - let before = session.dataRevision - - registry.evict(for: session.id) - let afterEvict = session.dataRevision - registry.evict(for: session.id) - - #expect(afterEvict > before) - #expect(session.dataRevision == afterEvict) - #expect(session.tableRows.index(of: .existing(0)) == nil) - } - // MARK: - Stale mark private static func rowsChange(at instant: ContinuousClock.Instant = .now) -> TableFreshness.Change { TableFreshness.Change(extent: .rows, at: instant) } - @Test("Marking a tab stale keeps its rows, where evicting drops them") + @Test("Marking a tab stale keeps its rows") func markingStaleKeepsTheRows() { let registry = TabSessionRegistry() let stale = UUID() - let evicted = UUID() registry.setTableRows(makeRows(["a", "b", "c"]), for: stale) - registry.setTableRows(makeRows(["a", "b", "c"]), for: evicted) registry.recordChange(Self.rowsChange(), for: stale) - registry.evict(for: evicted) #expect(registry.isStale(stale)) #expect(registry.tableRows(for: stale).rows.count == 3) - #expect(!registry.isEvicted(stale)) - #expect(registry.tableRows(for: evicted).rows.isEmpty) } - @Test("A tab whose last result was empty can be marked stale, which eviction refuses") + @Test("A tab whose last result was empty can be marked stale") func emptyResultCanBeMarkedStale() { let registry = TabSessionRegistry() let tabId = UUID() registry.setTableRows(makeRows([]), for: tabId) - registry.evict(for: tabId) registry.recordChange(Self.rowsChange(), for: tabId) - #expect(!registry.isEvicted(tabId)) #expect(registry.isStale(tabId)) } diff --git a/apple/TableProTests/Models/Query/TabSessionTests.swift b/apple/TableProTests/Models/Query/TabSessionTests.swift index f166846bd2..ff9bda4a49 100644 --- a/apple/TableProTests/Models/Query/TabSessionTests.swift +++ b/apple/TableProTests/Models/Query/TabSessionTests.swift @@ -20,7 +20,6 @@ struct TabSessionTests { #expect(session.tableRows.rows.isEmpty) #expect(session.tableRows.columns.isEmpty) - #expect(session.isEvicted == false) #expect(session.dataRevision == 0) #expect(session.rowSetRevision == 0) } diff --git a/apple/TableProTests/Models/Query/TableRowsRowLocatorTests.swift b/apple/TableProTests/Models/Query/TableRowsRowLocatorTests.swift index 08b699f698..9867fafb36 100644 --- a/apple/TableProTests/Models/Query/TableRowsRowLocatorTests.swift +++ b/apple/TableProTests/Models/Query/TableRowsRowLocatorTests.swift @@ -69,11 +69,4 @@ struct TableRowsRowLocatorTests { #expect(rows.rowLocators.isEmpty) #expect(table(nil).rowLocators.isEmpty) } - - @Test("Releasing the rows releases their locators") - func discardClears() { - var rows = table(["A", "B", "C"]) - rows.discardRowsKeepingMetadata() - #expect(rows.rowLocators.isEmpty) - } } diff --git a/apple/TableProTests/Models/Query/TableRowsTests.swift b/apple/TableProTests/Models/Query/TableRowsTests.swift index 601fb699b8..0d86475188 100644 --- a/apple/TableProTests/Models/Query/TableRowsTests.swift +++ b/apple/TableProTests/Models/Query/TableRowsTests.swift @@ -173,23 +173,6 @@ struct TableRowsIDLookupTests { let insertedID = table.rows[0].id #expect(table.row(withID: insertedID)?.values == ["v"]) } - - @Test("discardRowsKeepingMetadata releases rows and their ID index") - func discardRowsKeepingMetadataClearsIndex() { - var table = TableRows.from( - queryRows: [["a"]], - columns: ["c1"], - columnTypes: [.text(rawType: nil)] - ) - - table.discardRowsKeepingMetadata() - - #expect(table.rows.isEmpty) - #expect(table.index(of: .existing(0)) == nil) - #expect(table.row(withID: .existing(0)) == nil) - #expect(table.columns == ["c1"]) - #expect(table.columnTypes == [.text(rawType: nil)]) - } } struct TableRowsEditTests { diff --git a/apple/TableProTests/Views/Main/CatalogChangeWindowTests.swift b/apple/TableProTests/Views/Main/CatalogChangeWindowTests.swift index b6d9c1d808..fbeb76328f 100644 --- a/apple/TableProTests/Views/Main/CatalogChangeWindowTests.swift +++ b/apple/TableProTests/Views/Main/CatalogChangeWindowTests.swift @@ -182,7 +182,7 @@ struct CatalogChangeWindowTests { tabManager.selectedTabId = empty.id coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == .userInitiated) + #expect(coordinator.parkedTableLoad?.trigger == .userInitiated) } @Test("a rows change reloads the selected tab on that table when nothing of the user's is in it") @@ -247,7 +247,7 @@ struct CatalogChangeWindowTests { coordinator.applyObjectChange(Self.change(connection, name: "users", database: "shop", schema: nil, kind: .rows)) - #expect(coordinator.pendingLoadTrigger == .userInitiated) + #expect(coordinator.parkedTableLoad?.trigger == .userInitiated) } @Test("a structure change marks the structure of a tab on that table, and one holding staged edits owes it") diff --git a/apple/TableProTests/Views/Main/EvictionTests.swift b/apple/TableProTests/Views/Main/EvictionTests.swift deleted file mode 100644 index 351d593627..0000000000 --- a/apple/TableProTests/Views/Main/EvictionTests.swift +++ /dev/null @@ -1,357 +0,0 @@ -// -// EvictionTests.swift -// TableProTests -// -// Tests for cross-window tab eviction -// - -import Foundation -@testable import TablePro -import TableProPluginKit -@testable import TableProUtilities -import Testing - -@MainActor -struct EvictionTests { - private let fixtures = CoordinatorFixtures() - - private func makeCoordinator() -> (MainContentCoordinator, QueryTabManager) { - let fixture = fixtures.make() - return (fixture.coordinator, fixture.tabManager) - } - - private func addLoadedTab( - to coordinator: MainContentCoordinator, - tabManager: QueryTabManager, - tableName: String = "users" - ) throws { - try tabManager.addTableTab(tableName: tableName) - guard let index = tabManager.selectedTabIndex else { return } - let rows = TestFixtures.makeRows(count: 10) - let tabId = tabManager.tabs[index].id - let columns = ["id", "name", "email"] - let columnTypes: [ColumnType] = Array(repeating: .text(rawType: nil), count: columns.count) - let tableRows = TableRows.from(queryRows: rows.map { row in row.map(PluginCellValue.fromOptional) }, columns: columns, columnTypes: columnTypes) - coordinator.setActiveTableRows(tableRows, for: tabId) - let resultSet = ResultSet(label: tableName, tableRows: tableRows) - tabManager.mutate(at: index) { tab in - tab.display.resultSets = [resultSet] - tab.display.activeResultSetId = resultSet.id - tab.execution.lastExecutedAt = Date() - } - } - - @Test("evictInactiveRowData evicts background tabs without pending changes") - func evictsLoadedTabs() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - let backgroundResult = try #require(tabManager.tabs[0].display.activeResultSet) - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - #expect(backgroundResult.tableRows.rows.count == 10) - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == true) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.isEmpty) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).index(of: .existing(0)) == nil) - #expect(backgroundResult.tableRows.rows.isEmpty) - #expect(backgroundResult.tableRows.index(of: .existing(0)) == nil) - } - - @Test("eviction helper never evicts the selected tab") - func preservesSelectedTab() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let selectedTabId = try #require(tabManager.selectedTabId) - let selectedResult = try #require(tabManager.selectedTab?.display.activeResultSet) - let loadEpoch = try #require(tabManager.selectedTab?.loadEpoch) - - let didEvict = coordinator.evictReloadableTableRows(for: selectedTabId) - - #expect(didEvict == false) - #expect(coordinator.tabSessionRegistry.isEvicted(selectedTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: selectedTabId).rows.count == 10) - #expect(selectedResult.tableRows.rows.count == 10) - #expect(tabManager.selectedTab?.loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData skips tabs with pending changes") - func skipsTabsWithPendingChanges() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - - tabManager.tabs[0].pendingChanges.deletedRowIDs = [.existing(0)] - let loadEpoch = tabManager.tabs[0].loadEpoch - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - let tabId = tabManager.tabs[0].id - #expect(coordinator.tabSessionRegistry.isEvicted(tabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: tabId).rows.count == 10) - #expect(tabManager.tabs[0].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData skips tabs with pinned results") - func preservesPinnedResults() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - let pinnedResult = try #require(tabManager.tabs[0].display.activeResultSet) - pinnedResult.isPinned = true - let loadEpoch = tabManager.tabs[0].loadEpoch - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - #expect(pinnedResult.tableRows.rows.count == 10) - #expect(pinnedResult.tableRows.index(of: .existing(0)) == 0) - #expect(tabManager.tabs[0].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData skips table tabs that cannot auto-reload") - func skipsNonReloadableTableTabs() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - tabManager.tabs[0].execution.errorMessage = "connection lost" - let loadEpoch = tabManager.tabs[0].loadEpoch - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - #expect(tabManager.tabs[0].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData skips an executing table tab") - func skipsExecutingTableTab() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - let loadEpoch = tabManager.tabs[0].loadEpoch - let claim = coordinator.tabExecution.claim(backgroundTabId) - defer { _ = coordinator.tabExecution.settle(claim) } - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - #expect(tabManager.tabs[0].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData skips a table tab with an active load") - func skipsTableTabWithActiveLoad() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - let loadEpoch = tabManager.tabs[0].loadEpoch - let inFlight = Task { _ = try? await Task.sleep(for: .seconds(60)) } - coordinator.tableLoadTasks[backgroundTabId] = (UUID(), inFlight) - defer { - inFlight.cancel() - coordinator.tableLoadTasks[backgroundTabId] = nil - } - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - #expect(tabManager.tabs[0].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData skips tables loading rows", arguments: [false, true]) - func skipsTableTabLoadingRows(isLoadingMore: Bool) throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - let loadEpoch = tabManager.tabs[0].loadEpoch - tabManager.mutate(at: 0) { tab in - if isLoadingMore { - tab.pagination.isLoadingMore = true - } else { - tab.pagination.isLoading = true - } - } - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - #expect(tabManager.tabs[0].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData does not evict query results") - func preservesQueryResults() throws { - let (coordinator, tabManager) = makeCoordinator() - tabManager.addTab(initialQuery: "SELECT 1") - let queryIndex = try #require(tabManager.selectedTabIndex) - let queryTabId = tabManager.tabs[queryIndex].id - let queryRows = TableRows.from( - queryRows: [[.text("1")]], - columns: ["value"], - columnTypes: [.integer(rawType: nil)] - ) - coordinator.setActiveTableRows(queryRows, for: queryTabId) - let queryResult = ResultSet(label: "SELECT 1", tableRows: queryRows) - tabManager.mutate(at: queryIndex) { tab in - tab.display.resultSets = [queryResult] - tab.display.activeResultSetId = queryResult.id - tab.execution.lastExecutedAt = Date() - } - let loadEpoch = tabManager.tabs[queryIndex].loadEpoch - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(queryTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: queryTabId).rows.count == 1) - #expect(queryResult.tableRows.rows.count == 1) - #expect(tabManager.tabs[queryIndex].loadEpoch == loadEpoch) - } - - @Test("evictInactiveRowData preserves column metadata after eviction") - func preservesMetadataAfterEviction() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - let rows = coordinator.tabSessionRegistry.tableRows(for: backgroundTabId) - #expect(rows.columns == ["id", "name", "email"]) - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == true) - } - - @Test("evictInactiveRowData with no tabs is no-op") - func noTabsIsNoOp() { - let (coordinator, _) = makeCoordinator() - coordinator.evictInactiveRowData() - } - - @Test("eviction skips a table tab whose query is blank") - func skipsTableTabWithBlankQuery() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - tabManager.mutate(at: 0) { $0.content.query = " \n\t " } - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - } - - @Test("eviction skips a table tab running work that took no execution claim") - func skipsTableTabWithUnclaimedWork() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - let token = coordinator.tabExecution.beginUnclaimedWork(for: backgroundTabId) - defer { coordinator.tabExecution.endUnclaimedWork(token, for: backgroundTabId) } - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 10) - } - - @Test("metadata landing after eviction does not resurrect an empty tab") - func lateMetadataKeepsTabEvicted() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == true) - - coordinator.mutateActiveTableRows(for: backgroundTabId) { rows in - rows.columnEnumValues["status"] = ["active", "archived"] - return .columnsReplaced - } - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == true) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.isEmpty) - } - - @Test("rows arriving after eviction clear the evicted flag") - func reloadedRowsClearTheEvictedFlag() throws { - let (coordinator, tabManager) = makeCoordinator() - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "users") - let backgroundTabId = tabManager.tabs[0].id - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "orders") - - coordinator.evictInactiveRowData() - let reloaded = TableRows.from( - queryRows: [[.text("1")]], - columns: ["id"], - columnTypes: [.integer(rawType: nil)] - ) - coordinator.setActiveTableRows(reloaded, for: backgroundTabId) - - #expect(coordinator.tabSessionRegistry.isEvicted(backgroundTabId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: backgroundTabId).rows.count == 1) - } - - @Test("evictInactiveTabs keeps the newest tabs within the memory budget") - func budgetKeepsNewestTabs() throws { - let (coordinator, tabManager) = makeCoordinator() - let budget = MemoryPressureAdvisor.budgetForInactiveTabs() - let total = budget + 3 - var executedAt: [UUID: Date] = [:] - for index in 0.. 0) - for (position, tab) in evictable.enumerated() { - #expect(coordinator.tabSessionRegistry.isEvicted(tab.id) == (position < expectedEvicted)) - } - #expect(coordinator.tabSessionRegistry.isEvicted(selectedId) == false) - } - - @Test("evictInactiveTabs never evicts a tab it was told is active") - func budgetSkipsActiveTabs() throws { - let (coordinator, tabManager) = makeCoordinator() - let budget = MemoryPressureAdvisor.budgetForInactiveTabs() - for index in 0..<(budget + 3) { - try addLoadedTab(to: coordinator, tabManager: tabManager, tableName: "table_\(index)") - let tabIndex = try #require(tabManager.selectedTabIndex) - tabManager.mutate(at: tabIndex) { - $0.execution.lastExecutedAt = Date(timeIntervalSince1970: TimeInterval(1_000 + index)) - } - } - let selectedId = try #require(tabManager.selectedTabId) - let oldestId = tabManager.tabs[0].id - - coordinator.evictInactiveTabs(excluding: [selectedId, oldestId]) - - #expect(coordinator.tabSessionRegistry.isEvicted(oldestId) == false) - #expect(coordinator.tabSessionRegistry.tableRows(for: oldestId).rows.count == 10) - } -} diff --git a/apple/TableProTests/Views/Main/MainContentCoordinatorDisplayStateTests.swift b/apple/TableProTests/Views/Main/MainContentCoordinatorDisplayStateTests.swift index 8228adc933..ecec0f0675 100644 --- a/apple/TableProTests/Views/Main/MainContentCoordinatorDisplayStateTests.swift +++ b/apple/TableProTests/Views/Main/MainContentCoordinatorDisplayStateTests.swift @@ -119,20 +119,6 @@ struct MainContentCoordinatorDisplayStateTests { #expect(formatted.box(forID: .existing(0)) == nil) } - @Test("Evicting a tab's rows drops the text formatted from them") - func evictionDropsTheState() { - let (coordinator, tabManager) = makeCoordinator() - let first = addTableTab(to: tabManager, tableName: "orders") - let second = addTableTab(to: tabManager, tableName: "customers") - seedRows(coordinator, for: first.id) - seedRows(coordinator, for: second.id) - let formatted = cache(coordinator, first.id, rows: [0]) - - #expect(coordinator.evictReloadableTableRows(for: first.id)) - #expect(formatted.box(forID: .existing(0)) == nil) - #expect(coordinator.gridModel(forTab: first.id).rowCount == 0) - } - @Test("Closed tabs take their grid model with them") func closingATabPrunesTheState() { let (coordinator, tabManager) = makeCoordinator() diff --git a/apple/TableProTests/Views/Main/MainContentCoordinatorLazyLoadTests.swift b/apple/TableProTests/Views/Main/MainContentCoordinatorLazyLoadTests.swift index 8d1ddebb19..3251e31267 100644 --- a/apple/TableProTests/Views/Main/MainContentCoordinatorLazyLoadTests.swift +++ b/apple/TableProTests/Views/Main/MainContentCoordinatorLazyLoadTests.swift @@ -8,6 +8,7 @@ // and handleTabChange. // +import AppKit import Foundation import TableProPluginKit import Testing @@ -71,7 +72,7 @@ struct MainContentCoordinatorLazyLoadTests { func skipsWhenNoSelectedTab() { let (coordinator, _) = makeCoordinator() coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Returns early when selected tab is a query tab (not a table tab)") @@ -79,7 +80,7 @@ struct MainContentCoordinatorLazyLoadTests { let (coordinator, tabManager) = makeCoordinator() _ = addQueryTab(to: tabManager) coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Returns early when tab has an error message") @@ -92,7 +93,7 @@ struct MainContentCoordinatorLazyLoadTests { } tabManager.tabs[idx].execution.errorMessage = "boom" coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Returns early when tab query is whitespace-only") @@ -100,7 +101,7 @@ struct MainContentCoordinatorLazyLoadTests { let (coordinator, tabManager) = makeCoordinator() _ = addTableTab(to: tabManager, query: " ") coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Returns early when tab has fresh row data already loaded") @@ -115,7 +116,7 @@ struct MainContentCoordinatorLazyLoadTests { tabManager.tabs[idx].execution.lastExecutedAt = Date() coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) #expect(coordinator.tabSessionRegistry.tableRows(for: tabId).rows.count == 5) } @@ -133,7 +134,7 @@ struct MainContentCoordinatorLazyLoadTests { coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == .userInitiated) + #expect(coordinator.parkedTableLoad?.trigger == .userInitiated) #expect(coordinator.tabSessionRegistry.tableRows(for: tabId).rows.count == 5) } @@ -152,7 +153,7 @@ struct MainContentCoordinatorLazyLoadTests { coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Returns early when tab has pending edits in the change manager") @@ -167,7 +168,7 @@ struct MainContentCoordinatorLazyLoadTests { tabManager.tabs[idx].pendingChanges.deletedRowIDs = [.existing(0)] coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Returns early when a load Task is already registered for this tab") @@ -182,7 +183,7 @@ struct MainContentCoordinatorLazyLoadTests { #expect(coordinator.tableLoadTasks.count == 1) #expect(coordinator.tableLoadTasks[tabId] != nil) - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } /// The task slot stops answering once the load hands off to an execution: `executeQueryInternal` @@ -202,7 +203,7 @@ struct MainContentCoordinatorLazyLoadTests { #expect(coordinator.tabExecution.isCurrent(claim)) #expect(coordinator.tableLoadTasks[tabId] == nil) - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } /// Fetch All runs without claiming the tab, so the narrower `isExecuting` would let a second @@ -221,31 +222,31 @@ struct MainContentCoordinatorLazyLoadTests { coordinator.lazyLoadCurrentTabIfNeeded() #expect(coordinator.tableLoadTasks[tabId] == nil) - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } // MARK: - Connection guard - @Test("Sets pendingLoadTrigger when a fresh table tab is not connected") + @Test("Parks the load with its tab when a fresh table tab is not connected") func defersWhenDisconnected() { let (coordinator, tabManager) = makeCoordinator() - _ = addTableTab(to: tabManager) - coordinator.pendingLoadTrigger = nil + let tabId = addTableTab(to: tabManager) + coordinator.parkedTableLoad = nil coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == .userInitiated) + #expect(coordinator.parkedTableLoad == ParkedTableLoad(tabId: tabId, trigger: .userInitiated)) } - @Test("Carries the restore trigger into pendingLoadTrigger when disconnected") + @Test("Parks the restore trigger with its tab when disconnected") func carriesRestoreTriggerWhenDisconnected() { let (coordinator, tabManager) = makeCoordinator() - _ = addTableTab(to: tabManager) - coordinator.pendingLoadTrigger = nil + let tabId = addTableTab(to: tabManager) + coordinator.parkedTableLoad = nil coordinator.lazyLoadCurrentTabIfNeeded(trigger: .restore) - #expect(coordinator.pendingLoadTrigger == .restore) + #expect(coordinator.parkedTableLoad == ParkedTableLoad(tabId: tabId, trigger: .restore)) } @Test("A deferred restore tab does not load until its window becomes key") @@ -253,11 +254,11 @@ struct MainContentCoordinatorLazyLoadTests { let (coordinator, tabManager) = makeCoordinator() let tabId = addTableTab(to: tabManager) coordinator.deferredRestoreLoadTabId = tabId - coordinator.pendingLoadTrigger = nil + coordinator.parkedTableLoad = nil coordinator.lazyLoadCurrentTabIfNeeded(trigger: .restore) - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) #expect(coordinator.tableLoadTasks[tabId] == nil) } @@ -267,12 +268,12 @@ struct MainContentCoordinatorLazyLoadTests { let tabId = addTableTab(to: tabManager) coordinator.deferredRestoreLoadTabId = tabId coordinator.isKeyWindow = false - coordinator.pendingLoadTrigger = nil + coordinator.parkedTableLoad = nil coordinator.consumeDeferredRestoreLoadIfNeeded() #expect(coordinator.deferredRestoreLoadTabId == tabId) - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) #expect(coordinator.tableLoadTasks[tabId] == nil) } @@ -281,12 +282,12 @@ struct MainContentCoordinatorLazyLoadTests { let (coordinator, tabManager) = makeCoordinator() let tabId = addTableTab(to: tabManager) coordinator.deferredRestoreLoadTabId = tabId - coordinator.pendingLoadTrigger = nil + coordinator.parkedTableLoad = nil coordinator.handleWindowDidBecomeKey() #expect(coordinator.deferredRestoreLoadTabId == nil) - #expect(coordinator.pendingLoadTrigger == .restore) + #expect(coordinator.parkedTableLoad?.trigger == .restore) } /// A link or an agent can select another tab before the restored window comes forward. The @@ -303,10 +304,10 @@ struct MainContentCoordinatorLazyLoadTests { #expect(coordinator.deferredRestoreLoadTabId == nil) tabManager.selectedTabId = deferredId - coordinator.pendingLoadTrigger = nil + coordinator.parkedTableLoad = nil coordinator.lazyLoadCurrentTabIfNeeded() - #expect(coordinator.pendingLoadTrigger == .userInitiated) + #expect(coordinator.parkedTableLoad?.trigger == .userInitiated) } // MARK: - Idempotency @@ -326,7 +327,7 @@ struct MainContentCoordinatorLazyLoadTests { coordinator.lazyLoadCurrentTabIfNeeded() } #expect(coordinator.tabSessionRegistry.tableRows(for: tabId).rows.count == 4) - #expect(coordinator.pendingLoadTrigger == nil) + #expect(coordinator.parkedTableLoad == nil) } @Test("Clears an abandoned claim when no in-flight task remains") @@ -343,38 +344,7 @@ struct MainContentCoordinatorLazyLoadTests { coordinator.lazyLoadCurrentTabIfNeeded() #expect(coordinator.tabExecution.isExecuting(tabId) == false) - #expect(coordinator.pendingLoadTrigger == .userInitiated) - } - - // MARK: - loadEpoch bump triggers reload after eviction - - /// `.task(id:)` keys on the tab's own `loadEpoch`, so that is the one the eviction path has to - /// move. This used to assert on a copy held by the session that nothing reads, which would - /// have stayed green with the reload broken. - @Test("Evicting a background tab bumps the tab's loadEpoch so .task(id:) re-fires") - func evictionBumpsLoadEpoch() { - let (coordinator, tabManager) = makeCoordinator() - let background = addTableTab(to: tabManager, tableName: "orders") - seedRows(coordinator, for: background, rowCount: 7) - let foreground = addTableTab(to: tabManager, tableName: "customers") - tabManager.selectedTabId = foreground - guard let idx = tabManager.tabs.firstIndex(where: { $0.id == background }) else { - Issue.record("expected tab to exist") - return - } - tabManager.tabs[idx].execution.lastExecutedAt = Date() - let initialEpoch = tabManager.tabs[idx].loadEpoch - #expect(coordinator.tabSessionRegistry.tableRows(for: background).rows.count == 7) - - coordinator.evictInactiveRowData() - - guard let evictedTab = tabManager.tabs.first(where: { $0.id == background }) else { - Issue.record("expected tab to exist") - return - } - #expect(evictedTab.loadEpoch != initialEpoch) - #expect(coordinator.tabSessionRegistry.isEvicted(background) == true) - #expect(coordinator.tabSessionRegistry.isEvicted(foreground) == false) + #expect(coordinator.parkedTableLoad?.trigger == .userInitiated) } /// A claim with no task behind it is healed on the next lazy load. That heal never lowered the @@ -421,14 +391,91 @@ struct MainContentCoordinatorLazyLoadTests { #expect(coordinator.isKeyWindow == true) } - @Test("handleWindowDidBecomeKey cancels a pending eviction task") - func windowDidBecomeKeyCancelsEviction() { - let (coordinator, _) = makeCoordinator() - coordinator.handleWindowDidResignKey() - #expect(coordinator.evictionTask != nil) + @Test("Resigning and regaining key status runs no query") + func keyRoundTripRunsNothing() { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addTableTab(to: tabManager) + seedRows(coordinator, for: tabId) + tabManager.mutate(tabId: tabId) { $0.execution.lastExecutedAt = Date() } + coordinator.handleWindowDidResignKey() + #expect(coordinator.isKeyWindow == false) coordinator.handleWindowDidBecomeKey() - #expect(coordinator.evictionTask == nil) + #expect(coordinator.isKeyWindow == true) + #expect(coordinator.tabExecution.isAnyExecuting == false) + #expect(coordinator.tableLoadTasks[tabId] == nil) + #expect(coordinator.tabSessionRegistry.tableRows(for: tabId).rows.count == 3) } + + // MARK: - Loading once the connection comes back + + /// The parked load used to be one window-wide trigger, consumed against whatever tab was + /// selected, and a query tab took it as a Run. + @Test("A connection coming back never runs the selected query tab's statement") + func reconnectNeverRunsAQueryTab() { + let (coordinator, tabManager) = makeCoordinator() + let tableTab = addTableTab(to: tabManager) + coordinator.parkedTableLoad = ParkedTableLoad(tabId: tableTab, trigger: .userInitiated) + let queryTab = addQueryTab(to: tabManager, query: "DELETE FROM orders WHERE id = 7") + + coordinator.loadSelectedTableTabAfterConnect() + + #expect(coordinator.parkedTableLoad == nil) + #expect(coordinator.tabExecution.isBusy(queryTab) == false) + #expect(coordinator.queryTasks.hasTask(for: queryTab) == false) + } + + @Test("A parked trigger goes only to the tab that parked it") + func parkedTriggerStaysWithItsTab() { + let (coordinator, tabManager) = makeCoordinator() + let restored = addTableTab(to: tabManager) + coordinator.parkedTableLoad = ParkedTableLoad(tabId: restored, trigger: .restore) + + coordinator.loadSelectedTableTabAfterConnect() + #expect(coordinator.parkedTableLoad == ParkedTableLoad(tabId: restored, trigger: .restore)) + + let other = addTableTab(to: tabManager, tableName: "orders", query: "SELECT * FROM orders") + coordinator.loadSelectedTableTabAfterConnect() + #expect(coordinator.parkedTableLoad == ParkedTableLoad(tabId: other, trigger: .userInitiated)) + } + + // MARK: - A changed table under an open cell + + @Test("A tab whose table changed waits for an open cell to close before it reloads") + func changedTableWaitsForAnOpenCell() throws { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addTableTab(to: tabManager) + seedRows(coordinator, for: tabId) + tabManager.mutate(tabId: tabId) { $0.execution.lastExecutedAt = Date() } + coordinator.tabSessionRegistry.recordChange(TableFreshness.Change(extent: .rows, at: .now), for: tabId) + let editor = EditingGridView() + coordinator.gridModel(forTab: tabId).attach(editor) + let tab = try #require(tabManager.selectedTab) + + editor.isEditing = true + #expect(coordinator.canAutoLoadTableTab(tab) == false) + + editor.isEditing = false + #expect(coordinator.canAutoLoadTableTab(tab)) + } +} + +@MainActor +private final class EditingGridView: DataGridMountedView { + var isEditing = false + + func apply(_ change: DataGridChange) {} + func selectionDidChange() {} + func configurationDidChange() {} + func reveal(row: Int, displayColumn: Int?) {} + func revealColumn(_ displayColumn: Int) {} + func sortDidChange() {} + func summaryRulesDidChange() {} + func viewportSample() -> (row: Int, offset: CGFloat)? { nil } + func beginEditing(row: Int, displayColumn: Int) -> Bool { false } + func commitEditing() -> Bool { true } + func discardEditing() {} + func focus() -> Bool { false } + func anchor(forColumn displayColumn: Int) -> (view: NSView, rect: NSRect)? { nil } } diff --git a/apple/TableProTests/Views/Main/TableTabRowRetentionTests.swift b/apple/TableProTests/Views/Main/TableTabRowRetentionTests.swift new file mode 100644 index 0000000000..809cce7979 --- /dev/null +++ b/apple/TableProTests/Views/Main/TableTabRowRetentionTests.swift @@ -0,0 +1,145 @@ +// +// TableTabRowRetentionTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import TableProTestSupport +import TableProUtilities +import Testing + +@MainActor +struct TableTabRowRetentionTests { + private let fixtures = CoordinatorFixtures() + private static let rowCount = 10 + + private func makeCoordinator() -> (MainContentCoordinator, QueryTabManager) { + let fixture = fixtures.make() + return (fixture.coordinator, fixture.tabManager) + } + + private func addLoadedTableTab( + _ coordinator: MainContentCoordinator, + _ tabManager: QueryTabManager, + tableName: String + ) -> UUID { + var tab = QueryTab(title: tableName, query: "SELECT * FROM \(tableName)", tabType: .table, tableName: tableName) + tab.execution.lastExecutedAt = Date() + tabManager.tabs.append(tab) + tabManager.selectedTabId = tab.id + let rows = (0.. QueryTab { + try #require(tabManager.tabs.first { $0.id == id }) + } + + // The old budget eviction ran in the same deferred task as the cache prune, so the prune finishing means it would have run. + @Test("Switching through more table tabs than the cache budget keeps every tab's rows") + func switchingKeepsEveryTabsRows() async throws { + let (coordinator, tabManager) = makeCoordinator() + let ids = (0..<(GridDisplayCacheBudget.unmountedGrids() + 3)).map { + addLoadedTableTab(coordinator, tabManager, tableName: "table_\($0)") + } + let oldestCache = coordinator.gridModel(forTab: ids[0]).display.cache + oldestCache.setBox(RowDisplayBox(["formatted", nil]), forID: .existing(0)) + for id in ids.dropFirst() { + _ = coordinator.gridModel(forTab: id) + } + + let outgoing = ids[ids.count - 1] + let incoming = ids[ids.count - 2] + tabManager.selectedTabId = incoming + coordinator.handleTabChange(from: outgoing, to: incoming, tabs: tabManager.tabs) + try await waitUntil { oldestCache.box(forID: .existing(0)) == nil } + + for id in ids { + let current = try tab(id, in: tabManager) + #expect(coordinator.tabSessionRegistry.tableRows(for: id).rows.count == Self.rowCount) + #expect(coordinator.canAutoLoadTableTab(current) == false) + } + } + + @Test("The prune releases only formatted text of grids off screen and used longest ago") + func pruneReleasesOnlyOldUnmountedText() { + let (coordinator, tabManager) = makeCoordinator() + let ids = (0..<(GridDisplayCacheBudget.unmountedGrids() + 2)).map { + addLoadedTableTab(coordinator, tabManager, tableName: "table_\($0)") + } + let caches = ids.map { id in + let cache = coordinator.gridModel(forTab: id).display.cache + cache.setBox(RowDisplayBox(["formatted", nil]), forID: .existing(0)) + return cache + } + + coordinator.pruneGridDisplayCaches() + + #expect(caches[0].box(forID: .existing(0)) == nil) + for cache in caches.dropFirst() { + #expect(cache.box(forID: .existing(0)) != nil) + } + for id in ids { + #expect(coordinator.tabSessionRegistry.tableRows(for: id).rows.count == Self.rowCount) + } + } + + // Holds without waiting: the old 5 s eviction would need a fixed sleep to observe, which the test rules forbid. + @Test("Losing key status keeps every background table tab's rows") + func resigningKeyKeepsRows() throws { + let (coordinator, tabManager) = makeCoordinator() + let background = addLoadedTableTab(coordinator, tabManager, tableName: "orders") + let selected = addLoadedTableTab(coordinator, tabManager, tableName: "customers") + coordinator.handleWindowDidBecomeKey() + + coordinator.handleWindowDidResignKey() + + #expect(coordinator.isKeyWindow == false) + for id in [background, selected] { + let current = try tab(id, in: tabManager) + #expect(coordinator.tabSessionRegistry.tableRows(for: id).rows.count == Self.rowCount) + #expect(coordinator.canAutoLoadTableTab(current) == false) + } + } + + @Test("Rows cannot be edited while a load that replaces them is running") + func rowsAreReadOnlyWhileReplaced() throws { + let (coordinator, tabManager) = makeCoordinator() + let tabId = addLoadedTableTab(coordinator, tabManager, tableName: "orders") + tabManager.mutate(tabId: tabId) { $0.tableContext.isEditable = true } + #expect(coordinator.canEditActiveResult) + + let claim = coordinator.tabExecution.claim(tabId) + let loading = try tab(tabId, in: tabManager) + #expect(coordinator.canEditActiveResult == false) + #expect(coordinator.gridConfiguration(for: loading).isEditable == false) + + let settled = coordinator.tabExecution.settle(claim) + #expect(settled) + #expect(coordinator.canEditActiveResult) + } + + @Test("A tab whose table TablePro changed keeps its rows and reloads when shown") + func changedTableReloadsOverItsRows() throws { + let (coordinator, tabManager) = makeCoordinator() + let background = addLoadedTableTab(coordinator, tabManager, tableName: "orders") + _ = addLoadedTableTab(coordinator, tabManager, tableName: "customers") + + coordinator.tabSessionRegistry.recordChange( + TableFreshness.Change(extent: .rows, at: .now), + for: background + ) + + let current = try tab(background, in: tabManager) + #expect(coordinator.tabSessionRegistry.tableRows(for: background).rows.count == Self.rowCount) + #expect(coordinator.canAutoLoadTableTab(current)) + } +} From ae3cf94fa095399d47c044bdff220c30d6aa4fc2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 26 Oct 2026 17:46:38 +0700 Subject: [PATCH 2/2] chore(changelog): add fragments for #3848 --- apple/changelog.d/3848.fixed.1.md | 1 + apple/changelog.d/3848.fixed.2.md | 1 + apple/changelog.d/3848.fixed.3.md | 1 + apple/changelog.d/3848.fixed.md | 1 + 4 files changed, 4 insertions(+) create mode 100644 apple/changelog.d/3848.fixed.1.md create mode 100644 apple/changelog.d/3848.fixed.2.md create mode 100644 apple/changelog.d/3848.fixed.3.md create mode 100644 apple/changelog.d/3848.fixed.md diff --git a/apple/changelog.d/3848.fixed.1.md b/apple/changelog.d/3848.fixed.1.md new file mode 100644 index 0000000000..4e885668ad --- /dev/null +++ b/apple/changelog.d/3848.fixed.1.md @@ -0,0 +1 @@ +A table tab waiting for its connection could run the statement under the cursor in a query tab instead. diff --git a/apple/changelog.d/3848.fixed.2.md b/apple/changelog.d/3848.fixed.2.md new file mode 100644 index 0000000000..4fbc5d3d95 --- /dev/null +++ b/apple/changelog.d/3848.fixed.2.md @@ -0,0 +1 @@ +A table tab could reload under an open cell and drop what you were typing after a save in another window. diff --git a/apple/changelog.d/3848.fixed.3.md b/apple/changelog.d/3848.fixed.3.md new file mode 100644 index 0000000000..4a17efaa16 --- /dev/null +++ b/apple/changelog.d/3848.fixed.3.md @@ -0,0 +1 @@ +Edits made to rows while a query or refresh was still loading new ones were discarded when it finished. diff --git a/apple/changelog.d/3848.fixed.md b/apple/changelog.d/3848.fixed.md new file mode 100644 index 0000000000..a131958c66 --- /dev/null +++ b/apple/changelog.d/3848.fixed.md @@ -0,0 +1 @@ +Table tabs ran their query again when you switched back to them or returned to TablePro from another app.