Skip to content

Commit 4592f7c

Browse files
datlechindatlechin
committed
refactor: fix model, viewmodel, and theme architecture
Models: - MODEL-1.1: make buildBaseTableQuery accept dependencies as parameters - MODEL-1.3: add isEvicted computed property, document RowBuffer aliasing - MODEL-1.4: create AdditionalFieldKey enum for type-safe dictionary keys - MODEL-1.5: extract FilterSQLPreviewGenerator from FilterStateManager - MODEL-1.9: verify SharedSidebarState cleanup on disconnect ViewModels: - VM-5.1: replace NSLog with os.Logger in SidebarViewModel - VM-5.2: document Binding storage contract on SidebarViewModel - VM-5.3: remove unnecessary [weak self] from @mainactor Tasks - APP-8.1: add @mainactor to @objc notification handlers - APP-8.2: document legacy notification migration path - VIEW-8.1: standardize on AppState.shared direct access Theme: - THEME-7.1: document ThemeEngine.shared View extension pattern - THEME-7.3: make importTheme async, dispatch reloadAvailableThemes to background
1 parent fc8f70a commit 4592f7c

17 files changed

Lines changed: 235 additions & 124 deletions

‎TablePro/AppDelegate+ConnectionHandler.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ extension AppDelegate {
270270

271271
// MARK: - SQL File Queue (drained by .databaseDidConnect)
272272

273-
@objc func handleDatabaseDidConnect() {
273+
@MainActor @objc func handleDatabaseDidConnect() {
274274
guard !queuedFileURLs.isEmpty else { return }
275275
let urls = queuedFileURLs
276276
queuedFileURLs.removeAll()

‎TablePro/AppDelegate+WindowConfig.swift‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ extension AppDelegate {
5858
return menu
5959
}
6060

61-
@objc func showWelcomeFromDock() {
61+
@MainActor @objc func showWelcomeFromDock() {
6262
openWelcomeWindow()
6363
}
6464

65-
@objc func connectFromDock(_ sender: NSMenuItem) {
65+
@MainActor @objc func connectFromDock(_ sender: NSMenuItem) {
6666
guard let connectionId = sender.representedObject as? UUID else { return }
6767
let connections = ConnectionStorage.shared.loadConnections()
6868
guard let connection = connections.first(where: { $0.id == connectionId }) else { return }
@@ -201,7 +201,7 @@ extension AppDelegate {
201201

202202
// MARK: - Window Notifications
203203

204-
@objc func windowDidBecomeKey(_ notification: Notification) {
204+
@MainActor @objc func windowDidBecomeKey(_ notification: Notification) {
205205
guard let window = notification.object as? NSWindow else { return }
206206
let windowId = ObjectIdentifier(window)
207207

@@ -241,7 +241,7 @@ extension AppDelegate {
241241
}
242242
}
243243

244-
@objc func windowWillClose(_ notification: Notification) {
244+
@MainActor @objc func windowWillClose(_ notification: Notification) {
245245
guard let window = notification.object as? NSWindow else { return }
246246

247247
configuredWindows.remove(ObjectIdentifier(window))
@@ -261,7 +261,7 @@ extension AppDelegate {
261261
}
262262
}
263263

264-
@objc func windowDidChangeOcclusionState(_ notification: Notification) {
264+
@MainActor @objc func windowDidChangeOcclusionState(_ notification: Notification) {
265265
guard let window = notification.object as? NSWindow,
266266
isHandlingFileOpen else { return }
267267

‎TablePro/ContentView.swift‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ struct ContentView: View {
3131
@State private var windowTitle: String
3232
@Environment(\.openWindow)
3333
private var openWindow
34-
@Environment(AppState.self) private var appState
35-
3634
private let storage = ConnectionStorage.shared
3735

3836
init(payload: EditorTabPayload?) {

‎TablePro/Core/Database/DatabaseManager.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ final class DatabaseManager {
324324

325325
/// Test-only: remove an injected session
326326
internal func removeSession(for connectionId: UUID) {
327+
SharedSidebarState.removeConnection(connectionId)
327328
removeSessionEntry(for: connectionId)
328329
}
329330
#endif

‎TablePro/Models/Connection/DatabaseConnection.swift‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,17 @@ enum ConnectionColor: String, CaseIterable, Identifiable, Codable {
362362

363363
// MARK: - Database Connection
364364

365+
/// Type-safe keys for the `additionalFields` dictionary
366+
private enum AdditionalFieldKey: String {
367+
case mongoAuthSource
368+
case mongoReadPreference
369+
case mongoWriteConcern
370+
case mssqlSchema
371+
case oracleServiceName
372+
case usePgpass
373+
case preConnectScript
374+
}
375+
365376
/// Model representing a database connection
366377
struct DatabaseConnection: Identifiable, Hashable {
367378
let id: UUID
@@ -384,38 +395,38 @@ struct DatabaseConnection: Identifiable, Hashable {
384395
var startupCommands: String?
385396

386397
var mongoAuthSource: String? {
387-
get { additionalFields["mongoAuthSource"]?.nilIfEmpty }
388-
set { additionalFields["mongoAuthSource"] = newValue ?? "" }
398+
get { additionalFields[AdditionalFieldKey.mongoAuthSource.rawValue]?.nilIfEmpty }
399+
set { additionalFields[AdditionalFieldKey.mongoAuthSource.rawValue] = newValue ?? "" }
389400
}
390401

391402
var mongoReadPreference: String? {
392-
get { additionalFields["mongoReadPreference"]?.nilIfEmpty }
393-
set { additionalFields["mongoReadPreference"] = newValue ?? "" }
403+
get { additionalFields[AdditionalFieldKey.mongoReadPreference.rawValue]?.nilIfEmpty }
404+
set { additionalFields[AdditionalFieldKey.mongoReadPreference.rawValue] = newValue ?? "" }
394405
}
395406

396407
var mongoWriteConcern: String? {
397-
get { additionalFields["mongoWriteConcern"]?.nilIfEmpty }
398-
set { additionalFields["mongoWriteConcern"] = newValue ?? "" }
408+
get { additionalFields[AdditionalFieldKey.mongoWriteConcern.rawValue]?.nilIfEmpty }
409+
set { additionalFields[AdditionalFieldKey.mongoWriteConcern.rawValue] = newValue ?? "" }
399410
}
400411

401412
var mssqlSchema: String? {
402-
get { additionalFields["mssqlSchema"]?.nilIfEmpty }
403-
set { additionalFields["mssqlSchema"] = newValue ?? "" }
413+
get { additionalFields[AdditionalFieldKey.mssqlSchema.rawValue]?.nilIfEmpty }
414+
set { additionalFields[AdditionalFieldKey.mssqlSchema.rawValue] = newValue ?? "" }
404415
}
405416

406417
var oracleServiceName: String? {
407-
get { additionalFields["oracleServiceName"]?.nilIfEmpty }
408-
set { additionalFields["oracleServiceName"] = newValue ?? "" }
418+
get { additionalFields[AdditionalFieldKey.oracleServiceName.rawValue]?.nilIfEmpty }
419+
set { additionalFields[AdditionalFieldKey.oracleServiceName.rawValue] = newValue ?? "" }
409420
}
410421

411422
var usePgpass: Bool {
412-
get { additionalFields["usePgpass"] == "true" }
413-
set { additionalFields["usePgpass"] = newValue ? "true" : "" }
423+
get { additionalFields[AdditionalFieldKey.usePgpass.rawValue] == "true" }
424+
set { additionalFields[AdditionalFieldKey.usePgpass.rawValue] = newValue ? "true" : "" }
414425
}
415426

416427
var preConnectScript: String? {
417-
get { additionalFields["preConnectScript"]?.nilIfEmpty }
418-
set { additionalFields["preConnectScript"] = newValue ?? "" }
428+
get { additionalFields[AdditionalFieldKey.preConnectScript.rawValue]?.nilIfEmpty }
429+
set { additionalFields[AdditionalFieldKey.preConnectScript.rawValue] = newValue ?? "" }
419430
}
420431

421432
init(
@@ -464,11 +475,11 @@ struct DatabaseConnection: Identifiable, Hashable {
464475
self.additionalFields = additionalFields
465476
} else {
466477
var fields: [String: String] = [:]
467-
if let v = mongoAuthSource { fields["mongoAuthSource"] = v }
468-
if let v = mongoReadPreference { fields["mongoReadPreference"] = v }
469-
if let v = mongoWriteConcern { fields["mongoWriteConcern"] = v }
470-
if let v = mssqlSchema { fields["mssqlSchema"] = v }
471-
if let v = oracleServiceName { fields["oracleServiceName"] = v }
478+
if let v = mongoAuthSource { fields[AdditionalFieldKey.mongoAuthSource.rawValue] = v }
479+
if let v = mongoReadPreference { fields[AdditionalFieldKey.mongoReadPreference.rawValue] = v }
480+
if let v = mongoWriteConcern { fields[AdditionalFieldKey.mongoWriteConcern.rawValue] = v }
481+
if let v = mssqlSchema { fields[AdditionalFieldKey.mssqlSchema.rawValue] = v }
482+
if let v = oracleServiceName { fields[AdditionalFieldKey.oracleServiceName.rawValue] = v }
472483
self.additionalFields = fields
473484
}
474485
}

‎TablePro/Models/Query/QueryTab.swift‎

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,18 @@ struct QueryTab: Identifiable, Equatable {
280280

281281
// Results — stored in a reference-type buffer to avoid CoW duplication
282282
// of large data when the struct is mutated (MEM-1 fix).
283-
// Note: When QueryTab is copied (struct CoW), copies share the same RowBuffer
284-
// instance. This is intentional — RowBuffer is a reference type specifically to
285-
// avoid duplicating large result arrays on every struct mutation.
283+
//
284+
// Shared reference semantics: When QueryTab is copied (struct CoW), copies share
285+
// the same RowBuffer instance. This is intentional — RowBuffer is a reference type
286+
// specifically to avoid duplicating large result arrays on every struct mutation.
287+
// Mutations to rowBuffer (e.g., evict/restore) affect all copies that share this
288+
// reference. Use `rowBuffer.copy()` when you need an independent snapshot.
286289
var rowBuffer: RowBuffer
287290

291+
/// Whether this tab's row data has been evicted to save memory.
292+
/// Convenience accessor for `rowBuffer.isEvicted`.
293+
var isEvicted: Bool { rowBuffer.isEvicted }
294+
288295
// Backward-compatible computed accessors for result data
289296
var resultColumns: [String] {
290297
get { rowBuffer.columns }
@@ -436,34 +443,43 @@ struct QueryTab: Identifiable, Equatable {
436443

437444
/// Build a clean base query for a table tab (no filters/sort).
438445
/// Used when restoring table tabs from persistence to avoid stale WHERE clauses.
439-
@MainActor static func buildBaseTableQuery(
446+
///
447+
/// - Parameters:
448+
/// - tableName: The table to query.
449+
/// - pageSize: Number of rows per page.
450+
/// - editorLanguage: The editor language for this database type.
451+
/// - paginationStyle: SQL pagination style (LIMIT vs OFFSET/FETCH).
452+
/// - offsetFetchOrderBy: ORDER BY clause for OFFSET/FETCH pagination.
453+
/// - pluginDriver: Optional plugin driver for NoSQL query building.
454+
/// - quoteIdentifier: Identifier quoting function.
455+
static func buildBaseTableQuery(
440456
tableName: String,
441-
databaseType: DatabaseType,
442-
quoteIdentifier: ((String) -> String)? = nil
457+
pageSize: Int,
458+
editorLanguage: EditorLanguage = .sql,
459+
paginationStyle: SQLDialectDescriptor.PaginationStyle = .limit,
460+
offsetFetchOrderBy: String = "ORDER BY (SELECT NULL)",
461+
pluginDriver: (any PluginDatabaseDriver)? = nil,
462+
quoteIdentifier: @escaping (String) -> String
443463
) -> String {
444-
let quote = quoteIdentifier ?? quoteIdentifierFromDialect(PluginManager.shared.sqlDialect(for: databaseType))
445-
let pageSize = AppSettingsManager.shared.dataGrid.defaultPageSize
446-
447464
// Use plugin's query builder when available (NoSQL drivers like etcd, Redis)
448-
if let pluginDriver = PluginManager.shared.queryBuildingDriver(for: databaseType),
465+
if let pluginDriver,
449466
let pluginQuery = pluginDriver.buildBrowseQuery(
450467
table: tableName, sortColumns: [], columns: [], limit: pageSize, offset: 0
451468
) {
452469
return pluginQuery
453470
}
454471

455-
switch PluginManager.shared.editorLanguage(for: databaseType) {
472+
switch editorLanguage {
456473
case .javascript:
457474
let escaped = tableName.replacingOccurrences(of: "\\", with: "\\\\").replacingOccurrences(of: "\"", with: "\\\"")
458475
return "db[\"\(escaped)\"].find({}).limit(\(pageSize))"
459476
case .bash:
460477
return "SCAN 0 MATCH * COUNT \(pageSize)"
461478
default:
462-
let quotedName = quote(tableName)
463-
switch PluginManager.shared.paginationStyle(for: databaseType) {
479+
let quotedName = quoteIdentifier(tableName)
480+
switch paginationStyle {
464481
case .offsetFetch:
465-
let orderBy = PluginManager.shared.offsetFetchOrderBy(for: databaseType)
466-
return "SELECT * FROM \(quotedName) \(orderBy) OFFSET 0 ROWS FETCH NEXT \(pageSize) ROWS ONLY;"
482+
return "SELECT * FROM \(quotedName) \(offsetFetchOrderBy) OFFSET 0 ROWS FETCH NEXT \(pageSize) ROWS ONLY;"
467483
case .limit:
468484
return "SELECT * FROM \(quotedName) LIMIT \(pageSize);"
469485
}
@@ -585,7 +601,13 @@ final class QueryTabManager {
585601

586602
let pageSize = AppSettingsManager.shared.dataGrid.defaultPageSize
587603
let query = QueryTab.buildBaseTableQuery(
588-
tableName: tableName, databaseType: databaseType, quoteIdentifier: quoteIdentifier
604+
tableName: tableName,
605+
pageSize: pageSize,
606+
editorLanguage: PluginManager.shared.editorLanguage(for: databaseType),
607+
paginationStyle: PluginManager.shared.paginationStyle(for: databaseType),
608+
offsetFetchOrderBy: PluginManager.shared.offsetFetchOrderBy(for: databaseType),
609+
pluginDriver: PluginManager.shared.queryBuildingDriver(for: databaseType),
610+
quoteIdentifier: quoteIdentifier ?? quoteIdentifierFromDialect(PluginManager.shared.sqlDialect(for: databaseType))
589611
)
590612
var newTab = QueryTab(
591613
title: tableName,
@@ -607,7 +629,13 @@ final class QueryTabManager {
607629
) {
608630
let pageSize = AppSettingsManager.shared.dataGrid.defaultPageSize
609631
let query = QueryTab.buildBaseTableQuery(
610-
tableName: tableName, databaseType: databaseType, quoteIdentifier: quoteIdentifier
632+
tableName: tableName,
633+
pageSize: pageSize,
634+
editorLanguage: PluginManager.shared.editorLanguage(for: databaseType),
635+
paginationStyle: PluginManager.shared.paginationStyle(for: databaseType),
636+
offsetFetchOrderBy: PluginManager.shared.offsetFetchOrderBy(for: databaseType),
637+
pluginDriver: PluginManager.shared.queryBuildingDriver(for: databaseType),
638+
quoteIdentifier: quoteIdentifier ?? quoteIdentifierFromDialect(PluginManager.shared.sqlDialect(for: databaseType))
611639
)
612640
var newTab = QueryTab(
613641
title: tableName,
@@ -638,12 +666,16 @@ final class QueryTabManager {
638666
return false
639667
}
640668

669+
let pageSize = AppSettingsManager.shared.dataGrid.defaultPageSize
641670
let query = QueryTab.buildBaseTableQuery(
642671
tableName: tableName,
643-
databaseType: databaseType,
644-
quoteIdentifier: quoteIdentifier
672+
pageSize: pageSize,
673+
editorLanguage: PluginManager.shared.editorLanguage(for: databaseType),
674+
paginationStyle: PluginManager.shared.paginationStyle(for: databaseType),
675+
offsetFetchOrderBy: PluginManager.shared.offsetFetchOrderBy(for: databaseType),
676+
pluginDriver: PluginManager.shared.queryBuildingDriver(for: databaseType),
677+
quoteIdentifier: quoteIdentifier ?? quoteIdentifierFromDialect(PluginManager.shared.sqlDialect(for: databaseType))
645678
)
646-
let pageSize = AppSettingsManager.shared.dataGrid.defaultPageSize
647679

648680
// Build locally and write back once to avoid 14 CoW copies (UI-11).
649681
var tab = tabs[selectedIndex]

‎TablePro/Models/UI/FilterSQLPreviewGenerator.swift‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
//
2+
// FilterSQLPreviewGenerator.swift
3+
// TablePro
4+
//
5+
// SQL preview generation extracted from FilterStateManager
6+
//
7+
8+
import Foundation
9+
import TableProPluginKit
10+
11+
/// Generates SQL preview strings from filter state
12+
@MainActor
13+
struct FilterSQLPreviewGenerator {
14+
/// Generate preview SQL for the "SQL" button.
15+
/// Uses selected filters if any are selected, otherwise uses all valid filters.
16+
static func generatePreviewSQL(
17+
filters: [TableFilter],
18+
filterLogicMode: FilterLogicMode,
19+
databaseType: DatabaseType
20+
) -> String {
21+
guard let dialect = PluginManager.shared.sqlDialect(for: databaseType) else {
22+
return "-- Filters are applied natively"
23+
}
24+
let generator = FilterSQLGenerator(dialect: dialect)
25+
let filtersToPreview = selectFiltersForPreview(from: filters)
26+
27+
// If no valid filters but filters exist, show helpful message
28+
if filtersToPreview.isEmpty && !filters.isEmpty {
29+
let invalidCount = filters.count(where: { !$0.isValid })
30+
if invalidCount > 0 {
31+
return "-- No valid filters to preview\n-- Complete \(invalidCount) filter(s) by:\n-- • Selecting a column\n-- • Entering a value (if required)\n-- • Filling in second value for BETWEEN"
32+
}
33+
}
34+
35+
return generator.generateWhereClause(from: filtersToPreview, logicMode: filterLogicMode)
36+
}
37+
38+
/// Get filters to use for preview/application.
39+
/// If some (but not all) filters are selected, use only those.
40+
/// Otherwise use all valid filters (single-pass).
41+
static func selectFiltersForPreview(from filters: [TableFilter]) -> [TableFilter] {
42+
var valid: [TableFilter] = []
43+
var selectedValid: [TableFilter] = []
44+
for filter in filters where filter.isValid {
45+
valid.append(filter)
46+
if filter.isSelected { selectedValid.append(filter) }
47+
}
48+
// Only use selective mode when SOME (but not all) are selected
49+
if selectedValid.count == valid.count || selectedValid.isEmpty {
50+
return valid
51+
}
52+
return selectedValid
53+
}
54+
}

‎TablePro/Models/UI/FilterState.swift‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -353,41 +353,14 @@ final class FilterStateManager {
353353

354354
// MARK: - SQL Generation
355355

356-
/// Generate preview SQL for the "SQL" button
357-
/// Uses selected filters if any are selected, otherwise uses all valid filters
356+
/// Generate preview SQL for the "SQL" button.
357+
/// Uses selected filters if any are selected, otherwise uses all valid filters.
358358
func generatePreviewSQL(databaseType: DatabaseType) -> String {
359-
guard let dialect = PluginManager.shared.sqlDialect(for: databaseType) else {
360-
return "-- Filters are applied natively"
361-
}
362-
let generator = FilterSQLGenerator(dialect: dialect)
363-
let filtersToPreview = getFiltersForPreview()
364-
365-
// If no valid filters but filters exist, show helpful message
366-
if filtersToPreview.isEmpty && !filters.isEmpty {
367-
let invalidCount = filters.count(where: { !$0.isValid })
368-
if invalidCount > 0 {
369-
return "-- No valid filters to preview\n-- Complete \(invalidCount) filter(s) by:\n-- • Selecting a column\n-- • Entering a value (if required)\n-- • Filling in second value for BETWEEN"
370-
}
371-
}
372-
373-
return generator.generateWhereClause(from: filtersToPreview, logicMode: filterLogicMode)
374-
}
375-
376-
/// Get filters to use for preview/application
377-
/// If some (but not all) filters are selected, use only those
378-
/// Otherwise use all valid filters (single-pass)
379-
private func getFiltersForPreview() -> [TableFilter] {
380-
var valid: [TableFilter] = []
381-
var selectedValid: [TableFilter] = []
382-
for filter in filters where filter.isValid {
383-
valid.append(filter)
384-
if filter.isSelected { selectedValid.append(filter) }
385-
}
386-
// Only use selective mode when SOME (but not all) are selected
387-
if selectedValid.count == valid.count || selectedValid.isEmpty {
388-
return valid
389-
}
390-
return selectedValid
359+
FilterSQLPreviewGenerator.generatePreviewSQL(
360+
filters: filters,
361+
filterLogicMode: filterLogicMode,
362+
databaseType: databaseType
363+
)
391364
}
392365
}
393366

0 commit comments

Comments
 (0)