Skip to content

Commit a027cdd

Browse files
authored
refactor(datagrid): merge sortCache into coordinator.querySortCache (#937)
* refactor(datagrid): merge sortCache into coordinator.querySortCache The view-side @State sortCache (in MainEditorContentView) and the coordinator's @ObservationIgnored querySortCache stored byte-identical SortedRowsCache / QuerySortCacheEntry structs keyed by tab ID. Two write paths fed them: sync inline sort (small tables, ≤1000 rows) wrote to the view-side cache; async background sort (large tables) wrote to the coordinator-side cache. The read in sortedIDsForTab checked both. The split was historical, not architectural. Both caches share the same lifecycle (window/coordinator) and the row-mutation invalidations in MainContentCoordinator+RowOperations only cleared querySortCache — meaning a sync-sorted small table would silently return a stale sortedIDs after add/delete/undo. Bug fixed as a side effect of the merge. Drops: - private struct SortedRowsCache - @State var sortCache from MainEditorContentView - The two-step cache cleanup in onChange(of: tabStructureVersion) and onTeardown - The sortCache lookup branch in sortedIDsForTab The sync sort path now writes directly to coordinator.querySortCache. cleanupSortCache and the row-mutation invalidations cover both paths uniformly. -23 LOC. * test(datagrid): lock sort cache invalidation on row mutations Four tests cover the four mutation pathways through the now-merged querySortCache: - addNewRow invalidates the cache (was view-side sortCache before merge — silently stale) - duplicateSelectedRow invalidates - deleteSelectedRows invalidates when physically removing inserted rows - deleteSelectedRows preserves the cache on soft-delete of existing rows (sortedIDs reference rows by ID, soft-delete only changes appearance, not order) Documents the soft-delete-preserves invariant that emerged during test writing — guards a future contributor from "helpfully" invalidating on every delete and breaking sort persistence.
1 parent aa4a260 commit a027cdd

2 files changed

Lines changed: 100 additions & 33 deletions

File tree

TablePro/Views/Main/Child/MainEditorContentView.swift

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,6 @@ import AppKit
1010
import CodeEditSourceEditor
1111
import SwiftUI
1212

13-
/// Cache for sorted query result rows to avoid re-sorting on every SwiftUI body evaluation.
14-
/// Stores a permutation of `RowID` so the grid keeps the same display order even after
15-
/// inserts and deletes mutate the underlying TableRows storage.
16-
private struct SortedRowsCache {
17-
let sortedIDs: [RowID]
18-
let columnIndex: Int
19-
let direction: SortDirection
20-
let schemaVersion: Int
21-
}
22-
23-
/// Main editor content with tab bar and content switching
2413
struct MainEditorContentView: View {
2514
// MARK: - Dependencies
2615

@@ -58,10 +47,6 @@ struct MainEditorContentView: View {
5847
let onOffsetChange: (Int) -> Void
5948
let onPaginationGo: () -> Void
6049

61-
// MARK: - Sort Cache
62-
63-
@State private var sortCache: [UUID: SortedRowsCache] = [:]
64-
6550
@State private var cachedChangeManager: AnyChangeManager?
6651
@State private var erDiagramViewModels: [UUID: ERDiagramViewModel] = [:]
6752
@State private var serverDashboardViewModels: [UUID: ServerDashboardViewModel] = [:]
@@ -118,14 +103,7 @@ struct MainEditorContentView: View {
118103
favoriteDialogQuery = FavoriteDialogQuery(query: query)
119104
}
120105
.onChange(of: tabManager.tabStructureVersion) { _, _ in
121-
let newIds = tabManager.tabIds
122-
guard !sortCache.isEmpty || !erDiagramViewModels.isEmpty
123-
|| !serverDashboardViewModels.isEmpty else {
124-
coordinator.cleanupSortCache(openTabIds: Set(newIds))
125-
return
126-
}
127-
let openTabIds = Set(newIds)
128-
sortCache = sortCache.filter { openTabIds.contains($0.key) }
106+
let openTabIds = Set(tabManager.tabIds)
129107
coordinator.cleanupSortCache(openTabIds: openTabIds)
130108
erDiagramViewModels = erDiagramViewModels.filter { openTabIds.contains($0.key) }
131109
serverDashboardViewModels = serverDashboardViewModels.filter { openTabIds.contains($0.key) }
@@ -140,7 +118,6 @@ struct MainEditorContentView: View {
140118
refreshDataTabDelegateMutableRefs()
141119
coordinator.dataTabDelegate = dataTabDelegate
142120
coordinator.onTeardown = { [self] in
143-
sortCache.removeAll()
144121
cachedChangeManager = nil
145122
coordinator.dataTabDelegate = nil
146123
}
@@ -636,14 +613,6 @@ struct MainEditorContentView: View {
636613
return nil
637614
}
638615

639-
if let cached = sortCache[tab.id],
640-
cached.columnIndex == (tab.sortState.columnIndex ?? -1),
641-
cached.direction == tab.sortState.direction,
642-
cached.schemaVersion == tab.schemaVersion
643-
{
644-
return cached.sortedIDs
645-
}
646-
647616
let sortColumns = tab.sortState.columns
648617
let storageRows = resolvedRows.rows
649618
let sortedIndices = Array(storageRows.indices).sorted { idx1, idx2 in
@@ -666,7 +635,7 @@ struct MainEditorContentView: View {
666635
}
667636
let sortedIDs = sortedIndices.map { storageRows[$0].id }
668637

669-
sortCache[tab.id] = SortedRowsCache(
638+
coordinator.querySortCache[tab.id] = QuerySortCacheEntry(
670639
sortedIDs: sortedIDs,
671640
columnIndex: tab.sortState.columnIndex ?? -1,
672641
direction: tab.sortState.direction,
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
//
2+
// SortCacheInvalidationTests.swift
3+
// TableProTests
4+
//
5+
// Locks the contract that row mutations invalidate querySortCache for the
6+
// affected tab. Pre-merge, only the coordinator-side cache was invalidated;
7+
// the view-side @State sortCache stayed stale, so a sorted small table
8+
// returned out-of-date sortedIDs after add / undo / paste / delete. After
9+
// the merge there is one cache and these tests guard the invalidation set.
10+
//
11+
12+
import Foundation
13+
import Testing
14+
@testable import TablePro
15+
16+
@Suite("querySortCache invalidation on row mutations")
17+
@MainActor
18+
struct SortCacheInvalidationTests {
19+
private func makeCoordinator() -> (MainContentCoordinator, QueryTabManager, UUID) {
20+
let tabManager = QueryTabManager()
21+
let coordinator = MainContentCoordinator(
22+
connection: TestFixtures.makeConnection(),
23+
tabManager: tabManager,
24+
changeManager: DataChangeManager(),
25+
filterStateManager: FilterStateManager(),
26+
columnVisibilityManager: ColumnVisibilityManager(),
27+
toolbarState: ConnectionToolbarState()
28+
)
29+
tabManager.addTableTab(tableName: "users")
30+
let tabIndex = tabManager.selectedTabIndex ?? 0
31+
tabManager.tabs[tabIndex].tableContext.isEditable = true
32+
let tabId = tabManager.tabs[tabIndex].id
33+
return (coordinator, tabManager, tabId)
34+
}
35+
36+
private func seedCache(_ coordinator: MainContentCoordinator, for tabId: UUID) {
37+
coordinator.querySortCache[tabId] = QuerySortCacheEntry(
38+
sortedIDs: [.existing(0), .existing(1), .existing(2)],
39+
columnIndex: 1,
40+
direction: .ascending,
41+
schemaVersion: 0
42+
)
43+
}
44+
45+
private func seedRows(_ coordinator: MainContentCoordinator, for tabId: UUID, count: Int) {
46+
let columns = ["id", "name"]
47+
let rows = (0..<count).map { i in ["\(i)", "name\(i)"] }
48+
let columnTypes: [ColumnType] = Array(repeating: .text(rawType: nil), count: columns.count)
49+
let tableRows = TableRows.from(queryRows: rows, columns: columns, columnTypes: columnTypes)
50+
coordinator.setActiveTableRows(tableRows, for: tabId)
51+
}
52+
53+
@Test("addNewRow clears querySortCache for the tab")
54+
func addNewRowInvalidatesCache() {
55+
let (coordinator, _, tabId) = makeCoordinator()
56+
seedRows(coordinator, for: tabId, count: 3)
57+
seedCache(coordinator, for: tabId)
58+
59+
coordinator.addNewRow()
60+
61+
#expect(coordinator.querySortCache[tabId] == nil)
62+
}
63+
64+
@Test("deleteSelectedRows clears querySortCache when physically removing inserted rows")
65+
func physicalDeleteInvalidatesCache() {
66+
let (coordinator, _, tabId) = makeCoordinator()
67+
seedRows(coordinator, for: tabId, count: 3)
68+
coordinator.addNewRow()
69+
let insertedIndex = coordinator.tableRowsStore.tableRows(for: tabId).count - 1
70+
seedCache(coordinator, for: tabId)
71+
72+
coordinator.deleteSelectedRows(indices: [insertedIndex])
73+
74+
#expect(coordinator.querySortCache[tabId] == nil)
75+
}
76+
77+
@Test("deleteSelectedRows preserves querySortCache on soft delete of existing rows")
78+
func softDeletePreservesCache() {
79+
let (coordinator, _, tabId) = makeCoordinator()
80+
seedRows(coordinator, for: tabId, count: 5)
81+
seedCache(coordinator, for: tabId)
82+
83+
coordinator.deleteSelectedRows(indices: [0, 1])
84+
85+
#expect(coordinator.querySortCache[tabId] != nil)
86+
}
87+
88+
@Test("duplicateSelectedRow clears querySortCache for the tab")
89+
func duplicateRowInvalidatesCache() {
90+
let (coordinator, _, tabId) = makeCoordinator()
91+
seedRows(coordinator, for: tabId, count: 3)
92+
seedCache(coordinator, for: tabId)
93+
94+
coordinator.duplicateSelectedRow(index: 0)
95+
96+
#expect(coordinator.querySortCache[tabId] == nil)
97+
}
98+
}

0 commit comments

Comments
 (0)