Skip to content

Commit c9a17b1

Browse files
authored
refactor: Apple-pattern storage DI plus 3 product bug fixes (#955)
* fix(ssh): expand tilde in agent socket and identityAgent paths * fix(storage): persist group deletions before firing sync notification * fix(sql): throw when database dialect cannot be resolved * docs: add changelog entries for ssh, storage, dialect fixes * test: delete dead LIMIT-1 codegen test methods * test: update stale icon and SF Symbol expectations * test: add MSSQL plugin stub for unit test bundle * test: disable parallel execution in TablePro scheme * revert: re-enable parallel test execution * refactor(storage): inject UserDefaults and dependencies into GroupStorage * refactor(storage): inject UserDefaults into AppSettingsStorage * refactor(storage): inject file URL and dependencies into ConnectionStorage * refactor(sync): inject UserDefaults into SyncMetadataStorage * refactor(storage): inject database URL into QueryHistoryStorage * refactor(database): inject storage and plugin manager into DatabaseManager * refactor(plugins): inject plugin search URLs and UserDefaults into PluginManager * test: rewrite storage tests to use isolated instances * docs: add changelog entry for Apple-pattern singleton refactor * refactor(storage): inject database URL into SQLFavoriteStorage * refactor(sync): inject metadata storage into SyncChangeTracker * test: rewrite SQLFavoriteStorage and sync-aware tests with injected instances * refactor(storage): drop dead test-detection branch from QueryHistoryStorage default URL * docs: add changelog entry for SQLFavoriteStorage and SyncChangeTracker DI * fix(database): expose injected dependencies as internal for cross-file extensions * fix(storage): persist connection deletions before firing sync notification * docs: add changelog entry for connection delete fix and tighten storage refactor entries * docs: correct sync delete ordering invariant * style(plugins): add explicit internal access modifier to userPluginsDir * refactor(storage): convert SQLFavoriteStorage to actor
1 parent 26766e9 commit c9a17b1

63 files changed

Lines changed: 1445 additions & 1551 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2525

2626
### Changed
2727

28+
- Storage and sync singletons accept dependencies via init for test isolation, matching Apple's URLSession and UserDefaults convention. Production callers using `.shared` are unchanged. `SQLFavoriteStorage` is now an actor so its first access no longer blocks the main thread on SQLite setup.
2829
- Create Database dialog is now driver-driven. Each driver discovers its own valid options (PostgreSQL queries `pg_collation` and `pg_database`, MySQL/MariaDB query `information_schema.character_sets`/`collations`). The hardcoded macOS-flavored locale list is gone. Engines that don't support creation hide the Create button instead of failing on click.
2930
- Introduced TableRows, Row, and Delta value types in TablePro/Models/Query/ as the foundation for the data grid row model rewrite. No callers migrated yet (Phase C.1 of the DataGrid refactor).
3031
- DataGrid columns and cells refactored to use a persistent column pool and typed cell view hierarchy. CPU usage on table switch reduced significantly through proper NSTableView reuse pool retention.
@@ -72,6 +73,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
7273
- Native Search Field focus regression when clearing text
7374
- PostgreSQL Create Database failed with `new collation incompatible with template database` on glibc-initialized servers (#927). Encodings, collations, and the `template1` defaults are now read from the server. `LC_CTYPE` mirrors `LC_COLLATE`, and `TEMPLATE template0` is added automatically when the chosen collation differs from `template1.datcollate`.
7475
- Redshift Create Database emitted PostgreSQL `LC_COLLATE` syntax which is invalid Redshift grammar. Now emits `COLLATE { CASE_SENSITIVE | CASE_INSENSITIVE }`.
76+
- Expand tilde in SSH agent socket and `IdentityAgent` paths so 1Password and similar agents work when configured with `~/...` paths.
77+
- Persist group deletions before firing the sync notification, fixing a race that could re-upload deleted groups via iCloud.
78+
- Persist connection deletions before firing the sync notification, fixing the same race for deleted connections.
79+
- Refuse to generate SQL when the database dialect cannot be resolved, instead of silently emitting unquoted identifiers.
7580

7681
## [0.36.0] - 2026-04-27
7782

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ When adding a new method to the driver protocol: add to `PluginDatabaseDriver` (
123123

124124
These have caused real bugs when violated:
125125

126-
**Sync delete ordering**: In `ConnectionStorage` (and all storage classes), `SyncChangeTracker.markDeleted()` must be called BEFORE `saveConnections()`. The `markDeleted` call fires `postChangeNotification` which can trigger a sync — if `saveConnections` hasn't run yet, the file still has the deleted item and sync may re-add it.
126+
**Sync delete ordering**: In `ConnectionStorage` (and all storage classes), `SyncChangeTracker.markDeleted()` must be called AFTER `saveConnections()`. The `markDeleted` call fires `postChangeNotification` which can trigger a sync. If the file on disk still contains the deleted item when sync runs, it may re-upload the deleted record. Persist first, then notify.
127127

128128
**WelcomeViewModel tree rebuild**: The welcome screen renders `treeItems` (grouped/filtered), not `connections` directly. Every mutation to `connections` must call `rebuildTree()` afterward, or the UI won't update.
129129

TablePro/Core/ChangeTracking/DataChangeManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ final class DataChangeManager: ChangeManaging {
421421
)
422422
}
423423

424-
let generator = SQLStatementGenerator(
424+
let generator = try SQLStatementGenerator(
425425
tableName: tableName,
426426
columns: columns,
427427
primaryKeyColumns: primaryKeyColumns,

TablePro/Core/ChangeTracking/SQLStatementGenerator.swift

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,18 @@ struct SQLStatementGenerator {
3535
parameterStyle: ParameterStyle? = nil,
3636
dialect: SQLDialectDescriptor? = nil,
3737
quoteIdentifier: ((String) -> String)? = nil
38-
) {
38+
) throws {
3939
self.tableName = tableName
4040
self.columns = columns
4141
self.primaryKeyColumns = primaryKeyColumns
4242
self.databaseType = databaseType
4343
self.parameterStyle = parameterStyle ?? Self.defaultParameterStyle(for: databaseType)
44-
self.quoteIdentifierFn = quoteIdentifier ?? quoteIdentifierFromDialect(dialect)
44+
if let quoteIdentifier {
45+
self.quoteIdentifierFn = quoteIdentifier
46+
} else {
47+
let resolvedDialect = try resolveSQLDialect(for: databaseType, explicit: dialect)
48+
self.quoteIdentifierFn = quoteIdentifierFromDialect(resolvedDialect)
49+
}
4550
}
4651

4752
private static func defaultParameterStyle(for databaseType: DatabaseType) -> ParameterStyle {

TablePro/Core/Database/DatabaseManager+Health.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ extension DatabaseManager {
220220
// Resolve password for prompt-for-password connections
221221
var passwordOverride = activeSessions[sessionId]?.cachedPassword
222222
if session.connection.promptForPassword && passwordOverride == nil {
223-
let isApiOnly = PluginManager.shared.connectionMode(for: session.connection.type) == .apiOnly
223+
let isApiOnly = pluginManager.connectionMode(for: session.connection.type) == .apiOnly
224224
guard let prompted = await PasswordPromptHelper.prompt(
225225
connectionName: session.connection.name,
226226
isAPIToken: isApiOnly,

TablePro/Core/Database/DatabaseManager+SSH.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ extension DatabaseManager {
3939
keyPassphrase = SSHProfileStorage.shared.loadKeyPassphrase(for: profileId)
4040
totpSecret = SSHProfileStorage.shared.loadTOTPSecret(for: profileId)
4141
case .inline:
42-
storedSshPassword = ConnectionStorage.shared.loadSSHPassword(for: connection.id)
43-
keyPassphrase = ConnectionStorage.shared.loadKeyPassphrase(for: connection.id)
44-
totpSecret = ConnectionStorage.shared.loadTOTPSecret(for: connection.id)
42+
storedSshPassword = connectionStorage.loadSSHPassword(for: connection.id)
43+
keyPassphrase = connectionStorage.loadKeyPassphrase(for: connection.id)
44+
totpSecret = connectionStorage.loadTOTPSecret(for: connection.id)
4545
}
4646

4747
let sshPassword = sshPasswordOverride ?? storedSshPassword

TablePro/Core/Database/DatabaseManager+Sessions.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ extension DatabaseManager {
6969
if let cached = activeSessions[connection.id]?.cachedPassword {
7070
passwordOverride = cached
7171
} else {
72-
let isApiOnly = PluginManager.shared.connectionMode(for: connection.type) == .apiOnly
72+
let isApiOnly = pluginManager.connectionMode(for: connection.type) == .apiOnly
7373
guard let prompted = await PasswordPromptHelper.prompt(
7474
connectionName: connection.name,
7575
isAPIToken: isApiOnly,
@@ -150,7 +150,7 @@ extension DatabaseManager {
150150
}
151151

152152
// Save as last connection for "Reopen Last Session" feature
153-
AppSettingsStorage.shared.saveLastConnectionId(connection.id)
153+
appSettingsStorage.saveLastConnectionId(connection.id)
154154

155155
// Post notification for reliable delivery
156156
NotificationCenter.default.post(name: .databaseDidConnect, object: nil)
@@ -206,7 +206,7 @@ extension DatabaseManager {
206206
case .selectDatabaseFromLastSession:
207207
if resolvedConnection.database.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty,
208208
let adapter = driver as? PluginDriverAdapter,
209-
let savedDb = AppSettingsStorage.shared.loadLastDatabase(for: connection.id) {
209+
let savedDb = appSettingsStorage.loadLastDatabase(for: connection.id) {
210210
do {
211211
try await adapter.switchDatabase(to: savedDb)
212212
activeSessions[connection.id]?.currentDatabase = savedDb
@@ -237,7 +237,7 @@ extension DatabaseManager {
237237
}
238238
case .selectSchemaFromLastSession:
239239
if let schemaDriver = driver as? SchemaSwitchable,
240-
let savedSchema = AppSettingsStorage.shared.loadLastSchema(for: connection.id),
240+
let savedSchema = appSettingsStorage.loadLastSchema(for: connection.id),
241241
savedSchema != schemaDriver.currentSchema {
242242
do {
243243
try await schemaDriver.switchSchema(to: savedSchema)
@@ -267,15 +267,15 @@ extension DatabaseManager {
267267
session.currentDatabase = database
268268
session.currentSchema = nil
269269
}
270-
AppSettingsStorage.shared.saveLastSchema(nil, for: connectionId)
270+
appSettingsStorage.saveLastSchema(nil, for: connectionId)
271271
await reconnectSession(connectionId)
272272
} else if pm?.capabilities.supportsSchemaSwitching == true,
273273
let schemaDriver = driver as? SchemaSwitchable {
274274
try await schemaDriver.switchSchema(to: database)
275275
updateSession(connectionId) { session in
276276
session.currentSchema = database
277277
}
278-
AppSettingsStorage.shared.saveLastSchema(database, for: connectionId)
278+
appSettingsStorage.saveLastSchema(database, for: connectionId)
279279
return
280280
} else if let adapter = driver as? PluginDriverAdapter {
281281
try await adapter.switchDatabase(to: database)
@@ -288,7 +288,7 @@ extension DatabaseManager {
288288
}
289289
}
290290

291-
AppSettingsStorage.shared.saveLastDatabase(database, for: connectionId)
291+
appSettingsStorage.saveLastDatabase(database, for: connectionId)
292292
}
293293

294294
func switchSchema(to schema: String, for connectionId: UUID) async throws {
@@ -301,7 +301,7 @@ extension DatabaseManager {
301301
updateSession(connectionId) { session in
302302
session.currentSchema = schema
303303
}
304-
AppSettingsStorage.shared.saveLastSchema(schema, for: connectionId)
304+
appSettingsStorage.saveLastSchema(schema, for: connectionId)
305305
}
306306

307307
/// Switch to an existing session
@@ -367,7 +367,7 @@ extension DatabaseManager {
367367
} else {
368368
// No more sessions - clear current session and last connection ID
369369
currentSessionId = nil
370-
AppSettingsStorage.shared.saveLastConnectionId(nil)
370+
appSettingsStorage.saveLastConnectionId(nil)
371371
}
372372
}
373373
lifecycleLogger.info(

TablePro/Core/Database/DatabaseManager.swift

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ final class DatabaseManager {
1717
static let shared = DatabaseManager()
1818
internal static let logger = Logger(subsystem: "com.TablePro", category: "DatabaseManager")
1919

20+
@ObservationIgnored internal let connectionStorage: ConnectionStorage
21+
@ObservationIgnored internal let appSettingsStorage: AppSettingsStorage
22+
@ObservationIgnored internal let pluginManager: PluginManager
23+
2024
/// All active connection sessions
2125
internal(set) var activeSessions: [UUID: ConnectionSession] = [:] {
2226
didSet {
@@ -82,12 +86,20 @@ final class DatabaseManager {
8286
currentSession?.status ?? .disconnected
8387
}
8488

85-
internal init() {}
89+
internal init(
90+
connectionStorage: ConnectionStorage = .shared,
91+
appSettingsStorage: AppSettingsStorage = .shared,
92+
pluginManager: PluginManager = .shared
93+
) {
94+
self.connectionStorage = connectionStorage
95+
self.appSettingsStorage = appSettingsStorage
96+
self.pluginManager = pluginManager
97+
}
8698

8799
private func persistOpenConnectionIds() {
88-
let connections = ConnectionStorage.shared.loadConnections()
100+
let connections = connectionStorage.loadConnections()
89101
let activeKeys = Set(activeSessions.keys)
90102
let ids = connections.filter { activeKeys.contains($0.id) }.map(\.id)
91-
AppSettingsStorage.shared.saveLastOpenConnectionIds(ids)
103+
appSettingsStorage.saveLastOpenConnectionIds(ids)
92104
}
93105
}

TablePro/Core/Plugins/PluginManager.swift

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ final class PluginManager {
1616
private static let disabledPluginsKey = "com.TablePro.disabledPlugins"
1717
private static let legacyDisabledPluginsKey = "disabledPlugins"
1818

19+
@ObservationIgnored private let defaults: UserDefaults
20+
@ObservationIgnored private let builtInPluginsURL: URL?
21+
@ObservationIgnored internal let userPluginsDir: URL
22+
1923
internal(set) var plugins: [PluginEntry] = []
2024

2125
internal(set) var isInstalling = false
@@ -57,10 +61,8 @@ final class PluginManager {
5761

5862
private static let needsRestartKey = "com.TablePro.needsRestart"
5963

60-
var needsRestartStorage: Bool = UserDefaults.standard.bool(
61-
forKey: needsRestartKey
62-
) {
63-
didSet { UserDefaults.standard.set(needsRestartStorage, forKey: Self.needsRestartKey) }
64+
var needsRestartStorage: Bool {
65+
didSet { defaults.set(needsRestartStorage, forKey: Self.needsRestartKey) }
6466
}
6567

6668
var needsRestart: Bool { needsRestartStorage }
@@ -73,16 +75,9 @@ final class PluginManager {
7375

7476
internal(set) var pluginInstances: [String: any TableProPlugin] = [:]
7577

76-
private var builtInPluginsDir: URL? { Bundle.main.builtInPlugInsURL }
77-
78-
var userPluginsDir: URL {
79-
FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask)[0]
80-
.appendingPathComponent("TablePro/Plugins", isDirectory: true)
81-
}
82-
8378
var disabledPluginIds: Set<String> {
84-
get { Set(UserDefaults.standard.stringArray(forKey: Self.disabledPluginsKey) ?? []) }
85-
set { UserDefaults.standard.set(Array(newValue), forKey: Self.disabledPluginsKey) }
79+
get { Set(defaults.stringArray(forKey: Self.disabledPluginsKey) ?? []) }
80+
set { defaults.set(Array(newValue), forKey: Self.disabledPluginsKey) }
8681
}
8782

8883
static let logger = Logger(subsystem: "com.TablePro", category: "PluginManager")
@@ -91,7 +86,21 @@ final class PluginManager {
9186

9287
var queryBuildingDriverCache: [String: (any PluginDatabaseDriver)?] = [:]
9388

94-
private init() {}
89+
init(
90+
userDefaults: UserDefaults = .standard,
91+
builtInPluginsURL: URL? = Bundle.main.builtInPlugInsURL,
92+
userPluginsDir: URL = PluginManager.defaultUserPluginsDir()
93+
) {
94+
self.defaults = userDefaults
95+
self.builtInPluginsURL = builtInPluginsURL
96+
self.userPluginsDir = userPluginsDir
97+
self.needsRestartStorage = userDefaults.bool(forKey: Self.needsRestartKey)
98+
}
99+
100+
nonisolated static func defaultUserPluginsDir() -> URL {
101+
FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask)[0]
102+
.appendingPathComponent("TablePro/Plugins", isDirectory: true)
103+
}
95104

96105
// MARK: - Registry Metadata
97106

@@ -134,7 +143,6 @@ final class PluginManager {
134143
}
135144

136145
private func migrateDisabledPluginsKey() {
137-
let defaults = UserDefaults.standard
138146
if let legacy = defaults.stringArray(forKey: Self.legacyDisabledPluginsKey) {
139147
if defaults.stringArray(forKey: Self.disabledPluginsKey) == nil {
140148
defaults.set(legacy, forKey: Self.disabledPluginsKey)
@@ -317,7 +325,7 @@ final class PluginManager {
317325
}
318326
}
319327

320-
if let builtInDir = builtInPluginsDir {
328+
if let builtInDir = builtInPluginsURL {
321329
discoverPlugins(from: builtInDir, source: .builtIn)
322330
removeUserInstalledDuplicates(builtInDir: builtInDir)
323331
}

TablePro/Core/SSH/LibSSH2TunnelFactory.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,9 +508,9 @@ internal enum LibSSH2TunnelFactory {
508508
// Resolve agent socket: UI config > SSH config IdentityAgent > system default
509509
let socketPath: String?
510510
if !config.agentSocketPath.isEmpty {
511-
socketPath = config.agentSocketPath
511+
socketPath = SSHPathUtilities.expandTilde(config.agentSocketPath)
512512
} else if let agentPath = configEntry?.identityAgent, !agentPath.isEmpty {
513-
socketPath = agentPath
513+
socketPath = SSHPathUtilities.expandTilde(agentPath)
514514
} else {
515515
socketPath = nil
516516
}

0 commit comments

Comments
 (0)